-
Notifications
You must be signed in to change notification settings - Fork 16.3k
FIX: incorrect access of logical_date in google bigquery operator and google workflow operator #55110
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
providers/google/src/airflow/providers/google/cloud/hooks/bigquery.py
Outdated
Show resolved
Hide resolved
providers/google/src/airflow/providers/google/cloud/hooks/bigquery.py
Outdated
Show resolved
Hide resolved
MaksYermak
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.
@obarisk you have changed logical_date for workflow and bigquery operators. Is it all operators which use logical_date or do we have more?
providers/apache/hive/src/airflow/providers/apache/hive/operators/hive.py
Outdated
Show resolved
Hide resolved
|
@MaksYermak as far as I know, in after this PR, there's no |
providers/google/src/airflow/providers/google/cloud/hooks/bigquery.py
Outdated
Show resolved
Hide resolved
providers/google/src/airflow/providers/google/cloud/hooks/bigquery.py
Outdated
Show resolved
Hide resolved
providers/google/src/airflow/providers/google/cloud/operators/workflows.py
Outdated
Show resolved
Hide resolved
providers/google/src/airflow/providers/google/cloud/operators/workflows.py
Outdated
Show resolved
Hide resolved
providers/google/src/airflow/providers/google/cloud/operators/workflows.py
Outdated
Show resolved
Hide resolved
providers/apache/hive/src/airflow/providers/apache/hive/operators/hive.py
Outdated
Show resolved
Hide resolved
* fix: logical_date or run_after for google bigquery and workflow. use deprecation * update variable naming, warning and error messages
|
PR updated. I've update the description of the PR. |
|
@obarisk - do you need any help getting this over the finish line? Seems like there is another issue that would benefit from this being merged. |
|
The tests did pass. It's waiting for review now. cc: @jroachgolf84. Need the maintainers to review and merge. |
|
by the way, before it got merged I'm currently using workaround like the following. from dowhen import when
def patch_logical_date(_frame):
import pendulum # noqa
context = _frame.f_locals.get("context", {})
if not context:
print("no context found")
if context.get("logical_date") is None:
context["logical_date"] = pendulum.now("UTC")
_frame.f_locals["context"] = context
when(
GCSToBigQueryOperator.execute,
"""logical_date=context["logical_date"]"""
).do(patch_logical_date) |
providers/apache/hive/src/airflow/providers/apache/hive/operators/hive.py
Outdated
Show resolved
Hide resolved
providers/google/src/airflow/providers/google/cloud/hooks/bigquery.py
Outdated
Show resolved
Hide resolved
providers/google/src/airflow/providers/google/cloud/operators/bigquery.py
Show resolved
Hide resolved
providers/google/src/airflow/providers/google/cloud/sensors/cloud_composer.py
Outdated
Show resolved
Hide resolved
providers/google/src/airflow/providers/google/cloud/transfers/bigquery_to_gcs.py
Show resolved
Hide resolved
providers/google/src/airflow/providers/google/cloud/transfers/gcs_to_bigquery.py
Show resolved
Hide resolved
…ors/hive.py Co-authored-by: Wei Lee <weilee.rx@gmail.com>
…oud_composer.py Co-authored-by: Wei Lee <weilee.rx@gmail.com>
|
@Lee-W beside changing the signature of |
Lee-W
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.
a few nits. but I'm good with merging it. I'll keep it open for a few days in case others want to take a deeper look
providers/google/tests/unit/google/cloud/operators/test_workflows.py
Outdated
Show resolved
Hide resolved
providers/google/tests/unit/google/cloud/hooks/test_bigquery.py
Outdated
Show resolved
Hide resolved
|
Tests fail :( |
providers/apache/hive/src/airflow/providers/apache/hive/operators/hive.py
Outdated
Show resolved
Hide resolved
providers/google/src/airflow/providers/google/cloud/hooks/bigquery.py
Outdated
Show resolved
Hide resolved
providers/google/src/airflow/providers/google/cloud/operators/workflows.py
Outdated
Show resolved
Hide resolved
providers/google/tests/unit/google/cloud/operators/test_workflows.py
Outdated
Show resolved
Hide resolved
…uery.py Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
…ows.py Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
providers/google/tests/unit/google/cloud/hooks/test_bigquery.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Wei Lee <weilee.rx@gmail.com>
… google workflow operator (apache#55110) Co-authored-by: Wei Lee <weilee.rx@gmail.com> Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
… google workflow operator (apache#55110) Co-authored-by: Wei Lee <weilee.rx@gmail.com> Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
… google workflow operator (apache#55110) Co-authored-by: Wei Lee <weilee.rx@gmail.com> Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
… google workflow operator (apache#55110) Co-authored-by: Wei Lee <weilee.rx@gmail.com> Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
related: #53634 #55073 #55092
In airflow 3, logcial_date is not available for asset triggered DAG runs.
However, it was previously used for job ID generation in Bigquery and workflow ID for Workflows.
This PR proposes using dag_run.run_after as a substitute for asset triggered DAG runs.
In this PR, I propose to use dagrun.run_after for asset triggered dag run.
Summary of the PR
run_afterwith default value = None (for backwards compatibility)BigqueryHook.generate_job_id: for AIRFLOW_V_3_0_PLUS, use dag_run.run_after.What Wasn't FixedThe usage ofcontext["logical_date"]inproviders/google/cloud/sensors/cloud_composer.pyremains unchangeed.cc: @kaxil. this is related to #55073 but cover move on