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/05/27 10:03:31 UTC
[GitHub] [airflow] lwyszomi opened a new pull request, #23961: Support impersonation service account parameter for Dataflow runner
lwyszomi opened a new pull request, #23961:
URL: https://github.com/apache/airflow/pull/23961
The Apache beam team released a new version of SDK for python and JAVA and both started support for impersonation service account parameter.
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragement file, named `{pr_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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] mik-laj commented on a diff in pull request #23961: Support impersonation service account parameter for Dataflow runner
Posted by GitBox <gi...@apache.org>.
mik-laj commented on code in PR #23961:
URL: https://github.com/apache/airflow/pull/23961#discussion_r883500548
##########
airflow/providers/apache/beam/operators/beam.py:
##########
@@ -91,6 +92,13 @@ def __get_dataflow_pipeline_options(
pipeline_options[job_name_key] = job_name
if self.dataflow_config.service_account:
pipeline_options["serviceAccount"] = self.dataflow_config.service_account
+ if self.dataflow_support_impersonation and self.dataflow_config.impersonation_chain:
Review Comment:
Can you add tests to avoid regression?
--
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] mik-laj merged pull request #23961: Support impersonation service account parameter for Dataflow runner
Posted by GitBox <gi...@apache.org>.
mik-laj merged PR #23961:
URL: https://github.com/apache/airflow/pull/23961
--
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 #23961: Support impersonation service account parameter for Dataflow runner
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #23961:
URL: https://github.com/apache/airflow/pull/23961#discussion_r884220849
##########
airflow/providers/apache/beam/operators/beam.py:
##########
@@ -91,6 +92,13 @@ def __get_dataflow_pipeline_options(
pipeline_options[job_name_key] = job_name
if self.dataflow_config.service_account:
pipeline_options["serviceAccount"] = self.dataflow_config.service_account
+ if self.dataflow_support_impersonation and self.dataflow_config.impersonation_chain:
+ if isinstance(self.dataflow_config.impersonation_chain, list):
+ pipeline_options["impersonateServiceAccount"] = ",".join(
+ self.dataflow_config.impersonation_chain
+ )
+ else:
+ pipeline_options["impersonateServiceAccount"] = self.dataflow_config.impersonation_chain
Review Comment:
Instead of needing an extra flag, is it possible to always add the `impersonateServiceAccount` value? Or would it error if it’s present when not applicable?
--
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] mik-laj commented on a diff in pull request #23961: Support impersonation service account parameter for Dataflow runner
Posted by GitBox <gi...@apache.org>.
mik-laj commented on code in PR #23961:
URL: https://github.com/apache/airflow/pull/23961#discussion_r884329197
##########
airflow/providers/apache/beam/operators/beam.py:
##########
@@ -91,6 +92,13 @@ def __get_dataflow_pipeline_options(
pipeline_options[job_name_key] = job_name
if self.dataflow_config.service_account:
pipeline_options["serviceAccount"] = self.dataflow_config.service_account
+ if self.dataflow_support_impersonation and self.dataflow_config.impersonation_chain:
+ if isinstance(self.dataflow_config.impersonation_chain, list):
+ pipeline_options["impersonateServiceAccount"] = ",".join(
+ self.dataflow_config.impersonation_chain
+ )
+ else:
+ pipeline_options["impersonateServiceAccount"] = self.dataflow_config.impersonation_chain
Review Comment:
The behavior of apache beam can change from version to version, so I'd rather not add extra parameters when we don't need it.
--
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] lwyszomi commented on a diff in pull request #23961: Support impersonation service account parameter for Dataflow runner
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on code in PR #23961:
URL: https://github.com/apache/airflow/pull/23961#discussion_r883536665
##########
airflow/providers/apache/beam/operators/beam.py:
##########
@@ -91,6 +92,13 @@ def __get_dataflow_pipeline_options(
pipeline_options[job_name_key] = job_name
if self.dataflow_config.service_account:
pipeline_options["serviceAccount"] = self.dataflow_config.service_account
+ if self.dataflow_support_impersonation and self.dataflow_config.impersonation_chain:
Review Comment:
Done
--
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] mik-laj commented on a diff in pull request #23961: Support impersonation service account parameter for Dataflow runner
Posted by GitBox <gi...@apache.org>.
mik-laj commented on code in PR #23961:
URL: https://github.com/apache/airflow/pull/23961#discussion_r884329197
##########
airflow/providers/apache/beam/operators/beam.py:
##########
@@ -91,6 +92,13 @@ def __get_dataflow_pipeline_options(
pipeline_options[job_name_key] = job_name
if self.dataflow_config.service_account:
pipeline_options["serviceAccount"] = self.dataflow_config.service_account
+ if self.dataflow_support_impersonation and self.dataflow_config.impersonation_chain:
+ if isinstance(self.dataflow_config.impersonation_chain, list):
+ pipeline_options["impersonateServiceAccount"] = ",".join(
+ self.dataflow_config.impersonation_chain
+ )
+ else:
+ pipeline_options["impersonateServiceAccount"] = self.dataflow_config.impersonation_chain
Review Comment:
The behavior of apache beam can change from version to version, so I'd rather not add extra parameters when we don't need it. Especially when this is a new parameter and may not be stable yet.
--
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] lwyszomi commented on a diff in pull request #23961: Support impersonation service account parameter for Dataflow runner
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on code in PR #23961:
URL: https://github.com/apache/airflow/pull/23961#discussion_r884558374
##########
airflow/providers/apache/beam/operators/beam.py:
##########
@@ -91,6 +92,13 @@ def __get_dataflow_pipeline_options(
pipeline_options[job_name_key] = job_name
if self.dataflow_config.service_account:
pipeline_options["serviceAccount"] = self.dataflow_config.service_account
+ if self.dataflow_support_impersonation and self.dataflow_config.impersonation_chain:
+ if isinstance(self.dataflow_config.impersonation_chain, list):
+ pipeline_options["impersonateServiceAccount"] = ",".join(
+ self.dataflow_config.impersonation_chain
+ )
+ else:
+ pipeline_options["impersonateServiceAccount"] = self.dataflow_config.impersonation_chain
Review Comment:
For now Apache beam added this option for Java and Python SDK and if we pass this attr to the GO without new flag we will get error in the execution.
--
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