-
Notifications
You must be signed in to change notification settings - Fork 18
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
Adds Github Action for Airflow compute test #1348
Adds Github Action for Airflow compute test #1348
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.
@reviewers Ignore these changes. This is just a mirror copy of the other file because github actions needs the file to already exist on main in order to test out an action.
with: | ||
prefix: Airflow Compute | ||
|
||
# Sets it as an environmental variable. |
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'll uncomment before merging
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.
Actually will leave this commented out until I finish triaging all of the test case failures. Otherwise there will be a lot of noise generated. See https://linear.app/aqueducthq/issue/ENG-3009/investigate-airflow-integration-test-failures-and-triage-accordingly
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.
Mostly looks good! A few high / low level questions:
- confirming you are adding back other tests in
periodic-integration-tests.yml
? - should we publish pypi and start the test like how we do it in k8s / spark? What's the cadence of this test? If the same as k8s (and other periodic ones), we could add it as another job in
periodic-integration-tests
withpublish-pypi
as a dependency. - I see a step copying over DAG template. I'm wondering if we should do this step in
install_local.py -e
rather than just in testing env?
|
ad1c51a
to
c7c188f
Compare
@likawind Addressed everything, see reason why I am leaving slack notif commented out for now. |
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.
nice work!
Describe your changes and why you are making these changes
This PR adds a github action for running the Airflow compute tests once a week.
See https://github.com/aqueducthq/aqueduct/actions/runs/5043163833/jobs/9044608060 for an example run. Ignore the failures for now. These will all be filed as linear tasks that are beyond the scope of this task.
Related issue number (if any)
ENG 2599
Loom demo (if any)
Checklist before requesting a review
python3 scripts/run_linters.py -h
for usage).run_integration_test
: Runs integration testsskip_integration_test
: Skips integration tests (Should be used when changes are ONLY documentation/UI)