[GitHub] [airflow] mik-laj commented on a change in pull request #5634: [AIRFLOW-5012] Add typehints for gcp_*_hook.py

[GitBox <gi...@apache.org>]

mik-laj commented on a change in pull request #5634: [AIRFLOW-5012] Add typehints for gcp_*_hook.py
URL: https://github.com/apache/airflow/pull/5634#discussion_r308280206
 
 

 ##########
 File path: airflow/contrib/hooks/gcp_api_base_hook.py
 ##########
 @@ -212,49 +216,37 @@ def fallback_to_default_project_id(func):
         :return: result of the function call
         """
         @functools.wraps(func)
-        def inner_wrapper(self, *args, **kwargs):
+        def inner_wrapper(self: GoogleCloudBaseHook, *args, **kwargs) -> RT:
             if args:
                 raise AirflowException(
                     "You must use keyword arguments in this methods rather than"
                     " positional")
             if 'project_id' in kwargs:
-                kwargs['project_id'] = \
-                    self._get_project_id(kwargs['project_id'])  # pylint: disable=protected-access
+                kwargs['project_id'] = kwargs['project_id'] or self.project_id
             else:
-                kwargs['project_id'] = self._get_project_id(None)  # pylint: disable=protected-access
+                kwargs['project_id'] = self.project_id
             if not kwargs['project_id']:
                 raise AirflowException("The project id must be passed either as "
                                        "keyword project_id parameter or as project_id extra "
                                        "in GCP connection definition. Both are not set!")
             return func(self, *args, **kwargs)
         return inner_wrapper
 
-    def _get_project_id(self, project_id):
-        """
-        In case project_id is None, overrides it with default project_id from
-        the service account that is authorized.
-
-        :param project_id: project id to
-        :type project_id: str
-        :return: the project_id specified or default project id if project_id is None
-        """
-        return project_id if project_id else self.project_id
-
     class _Decorators:
         """A private inner class for keeping all decorator methods."""
 
         @staticmethod
-        def provide_gcp_credential_file(func):
+        def provide_gcp_credential_file(func: Callable[..., RT]) -> Callable[..., RT]:
             """
             Function decorator that provides a GOOGLE_APPLICATION_CREDENTIALS
             environment variable, pointing to file path of a JSON file of service
             account key.
             """
             @functools.wraps(func)
-            def wrapper(self, *args, **kwargs):
+            def wrapper(self: GoogleCloudBaseHook, *args, **kwargs) -> RT:
                 with tempfile.NamedTemporaryFile(mode='w+t') as conf_file:
-                    key_path = self._get_field('key_path', False)  # pylint: disable=protected-access
-                    keyfile_dict = self._get_field('keyfile_dict', False)  # pylint: disable=protected-access
+                    key_path = self._get_field('key_path', None)  # type: Optional[str]  # noqa: E501  #  pylint: disable=protected-access
 
 Review comment:
   I tried to limit unreleated changes in code. This mechanism requires a deeper thought. I hope that I will improve it when I deal with the AIRFLOW-4771 ticket.
   https://issues.apache.org/jira/browse/AIRFLOW-4771
   WDYT?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services