metaflow-tools icon indicating copy to clipboard operation
metaflow-tools copied to clipboard

Install Argo Events in Azure, setting version numbers in variables.tf

Open bbrandt opened this issue 1 year ago • 2 comments

Update of #19 with latest master merged and conflicts resolved. Moved version numbers for Argo and Argo events to variables.tf.

Draft PR #34 can be reviewed to verify the merge and conflict resolution was correct.

bbrandt avatar Feb 28 '23 15:02 bbrandt

@bbrandt Overall looks quite reasonable to me. Did you get a chance to use this PR in practice?

To set some expectations: Events triggering support is very close to merge. Once that is merged (perhaps by next week), I plan to also try out this PR as a kind of UAT (inclusive of exercising the said Metaflow eventing code). Then we would be in a good position to merge this PR.

jackie-ob avatar Mar 10 '23 00:03 jackie-ob

@jackie-ob

This is the testing I've done:

  • Create, delete, and create a cluster using this script about 3-4 times.
    • Passed testing
  • Change variables for argo and metaflow versions and rerun script to perform upgrades of argo and metaflow.
    • This worked the first 1 or 2 times, but I am currently hitting an issue with upgrading that I will need to look into before I'd feel comfortable merging this.
    • My hypothesis is that this issue may track back to what was stated in https://github.com/outerbounds/metaflow-tools/pull/19#discussion_r1006159562 about "standard TF resource state management"
  • Created flows in Argo from the Metaflow console app and triggered them to run
    • Passed testing
  • Tested with Romain's new @conda decorator from https://github.com/Netflix/metaflow-nflx-extensions
    • Passed testing

Not tested:

  • Sending events through Argo events and having it trigger a metaflow flow
    • This was on my to do list, but other work got in the way. Unfortunately, I may not get back to this for a bit, so I will mark this PR as draft and trust you to take it from here.

bbrandt avatar Mar 14 '23 15:03 bbrandt