potiuk commented on a change in pull request #4500: [AIRFLOW-3681] All GCP
operators have now optional GCP Project ID
URL: https://github.com/apache/airflow/pull/4500#discussion_r247371566
##########
File path: airflow/contrib/hooks/gcp_api_base_hook.py
##########
@@ -144,11 +145,46 @@ def _get_field(self, f, default=None):
key_path, etc. They get formatted as shown below.
"""
long_f = 'extra__google_cloud_platform__{}'.format(f)
- if long_f in self.extras:
+ if hasattr(self, 'extras') and long_f in self.extras:
return self.extras[long_f]
else:
return default
@property
def project_id(self):
return self._get_field('project')
+
+ def fallback_to_default_project_id(func):
+ """
+ Decorator that provides fallback for Google Cloud Platform project id.
If
+ the project is None it will be replaced with the project_id from the
+ service account the Hook is authenticated with. Project id can be
specified
+ either via project_id kwarg or via first parameter in positional args.
+ :param func: function to wrap
+ :return: result of the function call
+ """
+ @functools.wraps(func)
+ def inner_wrapper(self, *args, **kwargs):
+ if 'project_id' in kwargs:
+ kwargs['project_id'] =
self._get_project_id(kwargs['project_id'])
+ else:
+ # assume project_id is the first non-keyword parameter if
Review comment:
@kaxil - just wanted you to know that I am still working on it. I decided to
do it "properly" - i.e. change all project_ids to be keyword parameters and
have them in hook methods as last parameters with default = None. Thanks to our
automated system tests i will be able to test it for all operators e-2-e not
only unit tests (I am doing it as I am replacing service-by-service).
I also realised that since I am touching pretty much every hook method, it
will be safer to add unit tests for all the hooks. This way I can test if
project_id is indeed optional and whether it can be overridden for all hook
methods.
Lack of hook unit tests was a big missing point that we had in the
implementation, and It seems that i can do it rather quickly now - it's pretty
tedious but low-risk, high return.
I am done with almost all services - working on Spanner and ACL now (I have
working tests for all other services). I hope tomorrow I will send revised PR -
with literally hundreds more unit tests for hooks and truly optional
project_id. I hope it can make it to the release then :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
