Skip to content

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

Merged
merged 12 commits into from
Dec 2, 2020

Conversation

maciekgawron
Copy link
Contributor

@maciekgawron maciekgawron commented Nov 17, 2020

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

@maciekgawron maciekgawron requested review from leahecole and a team as code owners November 17, 2020 11:26
@product-auto-label product-auto-label bot added the samples Issues that are directly related to samples. label Nov 17, 2020
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Nov 17, 2020
Copy link
Collaborator

@leahecole leahecole left a 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 in tools, 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.
@maciekgawron
Copy link
Contributor Author

Hi Leah,

I moved the dag to workflows and added a test.
I managed to test it locally but it required locally running airflow+mysql.
I wonder if it is possible to test the DAGs in composer environment set in test-env.sh.

@leahecole leahecole added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 20, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 20, 2020
@leahecole
Copy link
Collaborator

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 :)

@leahecole leahecole added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 20, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 20, 2020
Copy link
Collaborator

@leahecole leahecole left a 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

@maciekgawron
Copy link
Contributor Author

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?

@leahecole
Copy link
Collaborator

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

@leahecole leahecole added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 25, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 25, 2020
@dinagraves dinagraves added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 25, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 25, 2020
keep_last_filters = context["params"].get("keep_last_filters")
keep_last_group_by = context["params"].get("keep_last_group_by")

logging.info("Configurations:")
Copy link
Contributor

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": ...."}.

@dinagraves dinagraves added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 2, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 2, 2020
@leahecole leahecole added automerge Merge the pull request once unit tests and other checks pass. kokoro:force-run Add this label to force Kokoro to re-run the tests. and removed automerge Merge the pull request once unit tests and other checks pass. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Dec 2, 2020
@leahecole
Copy link
Collaborator

working on trying to get the CI to run. Not sure why 3.6 is the only pending one 🤦‍♀️

@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 2, 2020
@leahecole leahecole merged commit 7ddc791 into GoogleCloudPlatform:master Dec 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement. samples Issues that are directly related to samples.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants