-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
feat(airflow) Override datajob external_url #9681
feat(airflow) Override datajob external_url #9681
Conversation
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.
Looks mostly reasonable to me. One question on implementation details
EDIT: Also, looks like you need to run the linter
...data-ingestion-modules/airflow-plugin/src/datahub_airflow_plugin/client/airflow_generator.py
Outdated
Show resolved
Hide resolved
metadata-ingestion-modules/airflow-plugin/src/datahub_airflow_plugin/lineage/datahub.py
Outdated
Show resolved
Hide resolved
...data-ingestion-modules/airflow-plugin/src/datahub_airflow_plugin/client/airflow_generator.py
Outdated
Show resolved
Hide resolved
b5551c0
to
e981ceb
Compare
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.
Looks like there's still lint issues
...data-ingestion-modules/airflow-plugin/src/datahub_airflow_plugin/client/airflow_generator.py
Outdated
Show resolved
Hide resolved
7876a9b
to
5083956
Compare
596065a
to
8a6efb0
Compare
8a6efb0
to
9861d61
Compare
Checklist
The current url link may not be that useful for the two reasons following, which is why this pr is brought up.
taskinstance page always times out for us due to the huge amount of DAGs we have.
taskinstance page is less informative compared to DAG page
This PR is about adding a configurable field such that users can choose the url of datajob between the page of taskinstance and grid on airflow.