-
Notifications
You must be signed in to change notification settings - Fork 6.6k
Add DAG for airflow metadb cleanup for Composer. #4993
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
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.
Hey Maciek! First of all, this is a great addition to our repo fo sure. A couple of questions to help make sure it is up to repo standards and also has the reach we want
- Could you add DAG testing like all of our other DAGs? This is an example of a test for a DAG that set variables, like this one would need. The imports might look a little different which leads to my next bullet point...
- Please put this directory in
workflows
. If you felt it should be intools
, I'd be fine with that too -tools
is actually empty - before we had in place image upgrades, there was a migration script, but that's been gone since before the WAW team worked on Composer :)
Once you've added a test, take a look at the test environment setup if you'd like to try running the tests locally.
most importantly, if you need any help with running tests, please ping me, or set up some time early California morning and I can help guide this PR to the finish.
Additionally some import order changes to be compliant with nox linter.
Hi Leah, I moved the dag to workflows and added a test. |
Hey! Sorry for delay. If you don't have a personal project you're testing against, the best bet is to use CI/CD. If you're not part of the GoogleCloudPlatform github org, the tests won't trigger. I just triggered them manually for you - if you ever need me to kick off a job, you can always ping me even if it's the middle of the night I'll kick it off first thing the next day :) |
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.
I noticed that you removed the README - is this going to be included or referenced in a tutorial? Or is the intent to share it with customers as needed? Just want to make sure this is discoverable and usable by folks who stumble upon it
I moved the content of README to the top of the DAG file. There is already a general README.rst in this catalogue, not sure if the other one will fit with the structure. Where do you usually put instructions for the other workflows? |
My bad - did not notice it had gone into the DAG. Most of these DAGs are embedded in tutorials on cloud.google.com, but some are not. Up to you for how you think folks should be pointed to it |
keep_last_filters = context["params"].get("keep_last_filters") | ||
keep_last_group_by = context["params"].get("keep_last_group_by") | ||
|
||
logging.info("Configurations:") |
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.
Why not make this a dict and then log the dict? eg {"Configurations: {"max_date": str(max_date), "enable_delete": ...."}.
working on trying to get the CI to run. Not sure why 3.6 is the only pending one 🤦♀️ |
Description
Added DAG for cleaning airflow metadb for Composer.
The DAG should be ready to use, just put the DAG in the Composer bucket.
The default retention period is 30days, it can be changed using airflow_db_cleanup__max_db_entry_age_in_days airflow variable.
The DAG is a fork from open-source repository https://github.com/teamclairvoyant/airflow-maintenance-dags/tree/master/db-cleanup
Checklist
nox -s py-3.6
(see Test Environment Setup)nox -s lint
(see Test Environment Setup)