-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Set task/dag labels for Dataproc Batch #46781
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
|
7a9ebdd to
83161b4
Compare
|
This looks good to me - @VladaZakharova let's have someone in the team review as well. |
|
Needs rebase :) |
|
Looks good to me! (Although needs rebase) @potiuk Can you please approve and merge, if it looks good to you? Thanks :) |
58530bc to
f522fa4
Compare
|
Thanks, updated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What problem does it solves?
is it a bug fix? feature?
please add meaningful description for this change
ah it was placed inside the template. now I see the description
providers/google/src/airflow/providers/google/cloud/operators/dataproc.py
Show resolved
Hide resolved
pierrejeambrun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beside Elad's comment, code looks good.
f522fa4 to
b8fdcb9
Compare
|
anything remaining @eladkal :)? |
d91b5fb to
3a25cd3
Compare
|
@eladkal can you please take a look again? I cannot merge this PR without your approval. Thanks. |
|
Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions. |
…pache#46781) * Set task/dag labels for Dataproc Batch * Adding airflow-dag-display-name label support
…pache#46781) * Set task/dag labels for Dataproc Batch * Adding airflow-dag-display-name label support
Set task/dag id labels for Dataproc Batch
Setting labels
airflow-dagandairflow-taskusing sanitized dag and task id. This should help the users to keep track of historical execution of dataproc serverless batches.User can use those labels to cluster dataproc batches by dags and tasks, perform historical analysis and navigate to the airflow source code.
During discussion, we considered alternative of using 'dag-display-name' but there is not enforcement for it to be a unique.
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in newsfragments.