Skip to content
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

Refactor model workflow to use external workflow defined in ccao-data/actions #59

Merged

Conversation

jeancochrane
Copy link
Contributor

@jeancochrane jeancochrane commented Nov 8, 2023

This PR updates the build-and-run-model workflow to point to the reusable build-and-run-batch-job workflow defined in ccao-data/actions#1. In the process, it removes a number of extraneous files that are now being managed by the external workflow:

  • The setup-terraform action
  • The poll_batch_job_script.sh script
  • The cleanup-model and cleanup-terraform workflows, which are now included as part of the cleanup step in build-and-run-batch-job.

Connects ccao-data/model-condo-avm#5.

Testing

  • For an example of a successful run of the build and test jobs of the build-and-run-model workflow, see this workflow run
    • Note that the run job fails, but only because I terminated the Batch job before it would complete; once the workflow gets to the point where it's polling the status of the Batch job, we can be confident that the test has passed, since nothing in the model pipeline itself has changed in this PR
  • For an example of a successful run of the cleanup job of the build-and-run-model workflow, see this workflow run

@jeancochrane jeancochrane force-pushed the jeancochrane/switch-to-external-actions-and-workflows branch 2 times, most recently from 950e448 to 953219f Compare November 8, 2023 22:20
@jeancochrane jeancochrane force-pushed the jeancochrane/switch-to-external-actions-and-workflows branch from e04d52e to 86729e1 Compare November 8, 2023 23:01
name: build-and-run-model

on:
pull_request:
types: [opened, reopened, synchronize, closed]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per the docs, opened, reopened, and closed are the default enabled event types when pull_request is enabled without a types attribute (as was the case before this PR). Now that the build-and-run-batch-job workflow supports running a cleanup step on the closed event (see docs here), we need to specify all of the previously-implicit event types in order to add closed to the list.

@@ -26,12 +27,11 @@ jobs:
# required in order to allow the reusable called workflow to push to
# GitHub Container Registry
packages: write
uses: ./.github/workflows/build-and-run-batch-job.yaml
uses: ccao-data/actions/.github/workflows/build-and-run-batch-job.yaml@master
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While it would be better to pin to a specific release of ccao-data/actions, my thinking is that it makes sense to pin to master while we're iterating quickly, and then pin to a tag once things are feeling stable.

Alternatively, we could pin to a specific commit SHA instead, but we won't know what that SHA is until ccao-data/actions#1 gets merged.

@jeancochrane jeancochrane changed the title Refactor model workflows to use ccao-data/actions workflows Refactor model workflow to use external workflow defined in ccao-data/actions Nov 9, 2023
@jeancochrane jeancochrane marked this pull request as ready for review November 9, 2023 20:13
@jeancochrane jeancochrane force-pushed the jeancochrane/switch-to-external-actions-and-workflows branch from a2b242c to 568b8aa Compare November 9, 2023 22:11
Copy link
Member

@dfsnow dfsnow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jeancochrane This is absolutely awesome! Super clean implementation. I can't wait to really test it out with some runs.

@jeancochrane jeancochrane merged commit bb5d2c7 into master Nov 14, 2023
3 of 4 checks passed
@jeancochrane jeancochrane deleted the jeancochrane/switch-to-external-actions-and-workflows branch November 14, 2023 22:50
jeancochrane added a commit that referenced this pull request Sep 26, 2024
* Pin build-and-run-batch-job to branch that fixes Terraform configs

* Delete terraform directory that has been unused since #59

* Lower resource requirements in build-and-run-model to test changing job definition

* Add missing ref, which is necessary when testing a different branch of ccao-data/actions

* Switch build-and-run-model params back to their usual values after testing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants