Skip to content

Tidy up build and run model pr#382

Merged
Damonamajor merged 7 commits intomasterfrom
Tidy-up-model-run-pr
Jun 3, 2025
Merged

Tidy up build and run model pr#382
Damonamajor merged 7 commits intomasterfrom
Tidy-up-model-run-pr

Conversation

@Damonamajor
Copy link
Contributor

@Damonamajor Damonamajor commented Jun 2, 2025

@Damonamajor Damonamajor marked this pull request as ready for review June 2, 2025 18:30
@Damonamajor Damonamajor self-assigned this Jun 2, 2025
Copy link
Member

@jeancochrane jeancochrane left a comment

Choose a reason for hiding this comment

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

One critical change! Re-request me once it's in and I can give a quick final approval.

uses: ccao-data/actions/.github/workflows/build-and-run-batch-job.yaml@main
with:
ref: Add-parameter-to-build-and-run-model-res-workflow-to-run-ingest-on-Batch
command: ${{ needs.parse-command.outputs.command }}
Copy link
Member

Choose a reason for hiding this comment

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

[Nitpick, required] This is likely why you had to unfreeze ingest to run this logic in the workflow -- without the command key, the reusable build-and-run-batch-job workflow will always fall back to the default (empty string), which will mean that the bash logic that decides whether to override the container command will never return an override. So basically, without command, repro_ingest will always be a no-op.

@Damonamajor Damonamajor requested a review from jeancochrane June 2, 2025 20:43
Copy link
Member

@jeancochrane jeancochrane left a comment

Choose a reason for hiding this comment

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

Good to go!

@Damonamajor Damonamajor merged commit 3a04b97 into master Jun 3, 2025
6 checks passed
@Damonamajor Damonamajor deleted the Tidy-up-model-run-pr branch June 3, 2025 17:49
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