-
Notifications
You must be signed in to change notification settings - Fork 91
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
Donotmerge this one - [DSRE-6] - Upgrade Airflow wtmo to 2.1 #1334
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 good to me
@@ -1,5 +1,5 @@ | |||
from airflow import DAG | |||
from airflow.operators.sensors import ExternalTaskSensor | |||
from airflow.sensors.external_task import ExternalTaskSensor |
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.
That reminds me, we'll also have to update the templates in bigquery-etl https://github.com/mozilla/bigquery-etl/blob/f26651463feaf5d48942d27467a0368afe6c5395/bigquery_etl/query_scheduling/templates/airflow_dag.j2#L4
@@ -1,7 +1,7 @@ | |||
import datetime | |||
|
|||
from airflow import DAG | |||
from airflow.contrib.hooks.aws_hook import AwsHook | |||
from airflow.providers.amazon.aws.hooks.base_aws import AwsBaseHook | |||
from operators.task_sensor import ExternalTaskCompletedSensor |
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.
Doesn't need to be part of this Pr, but once we switch to 2.1 we can drop the custom sensor here and just use the default ExternalTaskSensor
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.
[DSRE-200]
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.
aw, our github/jira integration was supposed to change that to a url link to https://mozilla-hub.atlassian.net/browse/DSRE-200
I guess it only works in the description field
AwsBaseHook(aws_conn_id=aws_conn_id, client_type='s3').get_credentials() if aws_conn_id else (), | ||
) | ||
if value is not None | ||
} |
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.
This is a bit verbose, but I think the explicitness is a good thing so that future developers working on this DAG can know the dev behavior just by looking at the content of the dag.
47377b2
to
cbf4a21
Compare
Sorry this is moving to #1377, I screwed up the branch somehow |
From: https://airflow.apache.org/announcements/
"May 21, 2021
I’m happy to announce that Apache Airflow 2.1.0 was just released. This one includes a raft of fixes and other small improvements, but some notable additions include:
Please note that this release no long includes the HTTP extra provider by default, as we discovered that it pulls in an LGPL dependency (via the requests module of all places) so it is now optional."
Cloud composer versions (just for reference)
https://cloud.google.com/composer/docs/concepts/versioning/composer-versions
More details DSRE-6
Commit messages (because these will eventually be squashed)
Except for our backported bigquery 1_10_2 code where new code also refs multiple conn ids.
The ltv dag references this backported code so is not changed.
Also this commit replaces GoogleCloudStorageDeleteOperator with the newer GCSDeleteObjectsOperator for 2.0
(in places I missed with a prev commit)