Page MenuHomePhabricator

[dagster-airflow] Fix k8s pod operator in dagster-airflow and get it under test

Authored by nate on Nov 15 2019, 12:08 AM.



well, it only took me 30 diffs, but this is now working and ready for review.

This diff fixes the Dagster k8s operator and provides a kind-based unit test, getting it under test.

To speed up this test, I've also moved the Docker image build step out to the nightly; it was getting extremely slow.

Test Plan


Diff Detail

R1 dagster
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
nate edited the summary of this revision. (Show Details)Nov 18 2019, 12:26 AM
nate added a reviewer: Restricted Project.
alangenfeld added inline comments.Nov 18 2019, 4:10 PM


is later before landing or something else? Seems worth taking care of before committing


i think this wont run if the tests fail - is that a problem?

alangenfeld requested changes to this revision.Nov 18 2019, 4:17 PM
alangenfeld added inline comments.

what exactly is in these images? I feel like this might introduce some weird corner case problems.

Either way I think this is a substantial enough change it should be pulled in to its own diff - it shouldn't block the rest of this diff


rm debug

This revision now requires changes to proceed.Nov 18 2019, 4:17 PM
nate marked 2 inline comments as done.Nov 19 2019, 5:19 AM
nate added inline comments.

trying depends_on in the main pipeline, maybe this has an easy solution


Moved to build image, removed


yeah I was thinking about this. not sure if there's a better way to do this cleanup. in the failure case, the "cluster" container will stay alive for the lifecycle of the host, definitely not ideal. any ideas?


thx, done

nate updated this revision to Diff 6707.Nov 19 2019, 5:20 AM
nate marked 2 inline comments as done.


nate planned changes to this revision.Nov 19 2019, 5:39 AM
nate added inline comments.

depends_on works!

TODO for self tomorrow:

  1. this should push/pull from ECR not docker hub (so that these are private)
  2. need to tag each build w/ a build ID so that the right images are used; rn everything is clobbering the latest tag
alangenfeld added inline comments.Nov 19 2019, 3:56 PM

one way is to move it to a bash script with a trap cleanup function

nate edited reviewers, added: max; removed: Restricted Project.Nov 19 2019, 4:12 PM
nate updated this revision to Diff 6714.Nov 19 2019, 6:13 PM

address TODOs

alangenfeld accepted this revision.Nov 19 2019, 11:45 PM



make this look like the rest for readability

This revision is now accepted and ready to land.Nov 19 2019, 11:45 PM
nate updated this revision to Diff 6728.Nov 20 2019, 12:19 AM