xnuinside commented on a change in pull request #3733: [AIRFLOW-491] Add cache 
parameter in BigQuery query method - with 'api_resource_configs'
URL: https://github.com/apache/incubator-airflow/pull/3733#discussion_r212533643
 
 

 ##########
 File path: airflow/contrib/hooks/bigquery_hook.py
 ##########
 @@ -587,15 +603,22 @@ def run_query(self,
         if use_legacy_sql is None:
             use_legacy_sql = self.use_legacy_sql
 
-        configuration = {
-            'query': {
+        configuration = deepcopy(api_resource_configs)
+
+        query_default =  {
                 'query': sql,
                 'useLegacySql': use_legacy_sql,
                 'maximumBillingTier': maximum_billing_tier,
                 'maximumBytesBilled': maximum_bytes_billed,
                 'priority': priority
             }
-        }
+
+        if not 'query' in configuration:
+            configuration['query'] = query_default
+        else:
+            for param in query_default:
+                if param not in configuration['query']:
 
 Review comment:
   I take apart it on two types - values what default by None and we raise the 
error as with sql. So priority will be on arg keyword, and only if it not exist 
- check api_resource and get key from it. Or you think need to raise error 
anyway if keys are duplicated? Or maybe just warning.
   Also, we can decide the priority of ways to passing args and if key provided 
with arg to method or operator - ignore it in api_ var and just write warning. 
   2-nd thing is keys what have default values like useLegacySql or priority. 
Because they are is not None by default we can not check what they were set by 
a user or not. But, I really don't see necessary for those default settings, 
because they default in google API and as I understand we do not need to 
provide them. 
   
   More I look on this config in the run_query, more want to change globally 
logic of creating conf for API request, or refactor part with configuration 
generating or forget idea with api_ var and just add useQueryCache param )) too 
many ifs...

----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to