You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2019/04/07 17:11:17 UTC

[GitHub] [airflow] mik-laj commented on a change in pull request #5054: [AIRFLOW-4255] Replace Discovery based api with client based for GCS

mik-laj commented on a change in pull request #5054: [AIRFLOW-4255] Replace Discovery based api with client based for GCS
URL: https://github.com/apache/airflow/pull/5054#discussion_r272843031
 
 

 ##########
 File path: airflow/contrib/hooks/gcs_hook.py
 ##########
 @@ -47,9 +45,11 @@ def get_conn(self):
         """
         Returns a Google Cloud Storage service object.
         """
-        http_authorized = self._authorize()
-        return build(
-            'storage', 'v1', http=http_authorized, cache_discovery=False)
+        if not self._conn:
+            self._conn = storage.Client(credentials=self._get_credentials(),
+                                        project=self.project_id)
 
 Review comment:
   In other hooks, `project_id` is a method parameter. In this implementation, user can only pass `project_id` as a connection configuration. This introduces inconsistencies. What steps should we take to unify these situations for all GCP operator?
   
   We have a 3 options:
   1) Specifying `project_id` in connection configuration.
   2) Specifying `project_id` in a method parameter with fallback to connection configuration
   3) Specifying `project_id` in a hook constructor parameter with fallback to connection configuration.
   
   The third variant does not appear anywhere, but it seems to me most expected. Initalizing parameters are not mixed with execution time parameters. `project_id` is a parameter that initialize client library. It don't execute a API call. 
   
   Probably the wrong place for this discussion, but we should take steps to use each operator and hook for GCP to be identical. 
   
   CC: @potiuk @antonimaciej 

----------------------------------------------------------------
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