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 2022/09/09 20:19:55 UTC

[GitHub] [airflow] patricker opened a new pull request, #26285: GCSToBigQueryOperator Resolve `max_id_key` job retrieval and xcom return

patricker opened a new pull request, #26285:
URL: https://github.com/apache/airflow/pull/26285

   closes: #26279
   closes: #26283
   
   Multiple issues with the `GCSToBigQueryOperator` operator feature `max_id_key`.
    - BQ Job was not retrievable after execution because of a change (about 2 years ago) with `@fallback_to_default_project_id`
    - Even if the value was retrievable, it was not being returned/set in XCOM, as specified in the parameter docs
   


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

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] potiuk merged pull request #26285: GCSToBigQueryOperator Resolve `max_id_key` job retrieval and xcom return

Posted by GitBox <gi...@apache.org>.
potiuk merged PR #26285:
URL: https://github.com/apache/airflow/pull/26285


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

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] uranusjr commented on a diff in pull request #26285: GCSToBigQueryOperator Resolve `max_id_key` job retrieval and xcom return

Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #26285:
URL: https://github.com/apache/airflow/pull/26285#discussion_r968156393


##########
tests/providers/google/cloud/transfers/test_gcs_to_bigquery.py:
##########
@@ -44,10 +44,13 @@ def test_execute_explicit_project(self, bq_hook):
 
         bq_hook.return_value.get_job.return_value.result.return_value = ('1',)
 
-        operator.execute(None)
+        result = operator.execute(None)
+
+        self.assertEqual(result, '1')

Review Comment:
   ```suggestion
           assert result == '1'
   ```
   
   Pytest-style assertions are preferred in new code.



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

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] uranusjr commented on a diff in pull request #26285: GCSToBigQueryOperator Resolve `max_id_key` job retrieval and xcom return

Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #26285:
URL: https://github.com/apache/airflow/pull/26285#discussion_r968155406


##########
airflow/providers/google/cloud/transfers/gcs_to_bigquery.py:
##########
@@ -315,16 +315,16 @@ def execute(self, context: 'Context'):
                 warnings.simplefilter("ignore", DeprecationWarning)
                 job_id = bq_hook.run_query(
                     sql=select_command,
+                    location=self.location,
                     use_legacy_sql=False,
                 )
-            row = list(bq_hook.get_job(job_id).result())
+            row = list(bq_hook.get_job(job_id=job_id, location=self.location).result())
             if row:
                 max_id = row[0] if row[0] else 0
                 self.log.info(
-                    'Loaded BQ data with max %s.%s=%s',
-                    self.destination_project_dataset_table,
-                    self.max_id_key,
-                    max_id,
+                    f'Loaded BQ data with max '
+                    f'{self.destination_project_dataset_table}.{self.max_id_key}={max_id}',
                 )

Review Comment:
   Log messages should not use f-string. Please revert this.



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

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] patricker commented on pull request #26285: GCSToBigQueryOperator Resolve `max_id_key` job retrieval and xcom return

Posted by GitBox <gi...@apache.org>.
patricker commented on PR #26285:
URL: https://github.com/apache/airflow/pull/26285#issuecomment-1243905806

   @uranusjr Thanks for the review. I've updated my code.


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

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org