e-galan commented on code in PR #38716:
URL: https://github.com/apache/airflow/pull/38716#discussion_r1555810282
##########
airflow/providers/apache/beam/operators/beam.py:
##########
@@ -364,11 +364,13 @@ def execute(self, context: Context):
def execute_sync(self, context: Context):
with ExitStack() as exit_stack:
- gcs_hook = GCSHook(gcp_conn_id=self.gcp_conn_id)
if self.py_file.lower().startswith("gs://"):
+ gcs_hook = GCSHook(gcp_conn_id=self.gcp_conn_id)
Review Comment:
Hello @zstrathe! You are right, the Google connection should be optional, as
it is needed only for particular use cases, and yours is not among them.
Your change on line 368 looks correct to me.
I would only suggest to update the unit tests that use this operator. In
particular, I think
[this](https://github.com/apache/airflow/blob/main/tests/providers/apache/beam/operators/test_beam.py#L142C5-L169C10)
and
[this](https://github.com/apache/airflow/blob/main/tests/providers/apache/beam/operators/test_beam.py#L251C5-L257C48)
tests will need some refactoring, but you can add new ones if you'd like.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
