build-cd v7: !update-configs: Prerelease to Model Configs Pipeline#321
build-cd v7: !update-configs: Prerelease to Model Configs Pipeline#321
build-cd v7: !update-configs: Prerelease to Model Configs Pipeline#321Conversation
!update-configs: Prerelease to Model Configs Pipelinebuild-cd v7: !update-configs: Prerelease to Model Configs Pipeline
beaa740 to
8ae2854
Compare
8ae2854 to
4fc262d
Compare
aidanheerdegen
left a comment
There was a problem hiding this comment.
Will probably have to do a pair-review at some stage, but I have some preliminary comments.
Co-authored-by: Aidan Heerdegen <[email protected]> Signed-off-by: Tommy Gatti <[email protected]>
… yq invocation, add tests
ba8c62b to
e844b47
Compare
|
Hey @dougiesquire - just getting back to this |
e0adb9d to
a06c2ac
Compare
…to their own steps
a06c2ac to
467bedb
Compare
467bedb to
597ccde
Compare
|
Whew, after a lot of testing I've got the matrixed jobs working @aidanheerdegen! Here are the two tests done to verify it works:
|
There was a problem hiding this comment.
Hey @dougiesquire - just getting back to this
!update-configscommand - how do you find the structure of the file forconfig/auto-configs-pr.json(under the Background section in the PR description)?
Looks great @CodeGat thanks. This would work particularly nicely if the repro check github comment reflected the pytest markers, so one could have e.g.
"profiles": {
"default": {
"configs_repo": "ACCESS-NRI/access-test-configs",
"configs": {
"dev-01deg_jra55_iaf": {
"checks": {
"repro": true <<< run all repro tests
}
},
"dev-01deg_jra55_ryf": {
"checks": {
"repro_historical": false <<< run historical repro tests
}
}
}
}
}
But that is a separate issue (there are other, possibly better, solutions to this in this issue).
|
Gotcha - I'd always treated the The linked issue would be good to sort out - but that will probably come later. I wonder if the "profiles": {
"default": {
"configs_repo": "ACCESS-NRI/access-test-configs",
"configs": {
"dev-01deg_jra55_iaf": {
"checks": {
"repro": true # run the default repro suite of tests (informed by the configs ci.json file)
"performance": true # run the default performance suite of tests (informed by the configs ci.json file)
}
},
"dev-01deg_jra55_ryf": {
"checks": {
"repro": "repro_historical and not slow" # run these specific markers for the repro suite of tests
"performance": "blah and not slow" # run these specific markers for the performance suite of tests
}
},
"dev-01deg_jra55_abc": {
"checks": {
"repro": false # don't run the repro suite of tests at all
"performance": false # don't run the performance suite of tests at all
}
}
}
}
}In the case you noted, rather than suites of tests, it would be a list of pytest markers, I suppose. But yeah, I think that can come later, since we need to get the |
aidanheerdegen
left a comment
There was a problem hiding this comment.
LGTM. But note I have not checked the python code carefully. I can do it, but don't currently have the time. If you need a thorough check might have to rope in someone else, but I am happy to proceed if you are.
|
It is much cleaner with the matrix approach, even if it does mean more CI steps. Also means we could move to multiple config repos fairly easily, as it is just a bigger matrix. |
Closes #320
Important
This PR is a major update to the deployment infrastructure. See below for the prerequisites for this repository to be able to merge this PR.
Background
There is still a bit of human overhead when opening Model Configs PRs based on new Prerelease builds.
In this PR, we allow for a new command for Model Deployment Repositories, one of the form:
This command will open PRs in a linked model configs repository based on a given profile. A profile is a set of configuration branches to create PRs from, defined in an MDRs
config/auto-configs-pr.jsonfile. This file is sourced from the MDR branch, not the default one, meaning users can make modifications in the prerelease PR as they see fit.This file looks something like:
{ "$schema": "https://raw.githubusercontent.com/ACCESS-NRI/schema/main/au.org.access-nri/model/deployment/config/auto-configs-pr/1-0-0.json", "profiles": { "default": { "configs_repo": "ACCESS-NRI/access-test-configs", "configs": { "dev-01deg_jra55_iaf": { "checks": { "repro": true } }, "dev-01deg_jra55_ryf": { "checks": { "repro": false } } } }, "qa-only": { "configs_repo": "ACCESS-NRI/access-test-configs", "configs": { "dev-01deg_jra55_iaf": { "checks": { "repro": false } }, "dev-01deg_jra55_ryf": { "checks": { "repro": false } } } } } }and specifies a profile name, a configs repository to source configs from, and a list of configs to generate PRs from. Furthermore, it will run the given checks on the new PR - for example,
!test repro.The PR
ci.ymlci-command-configs.yml, that handles the new!update-configscommandTesting
Testing of the syntax and outputs of the command was done in ACCESS-NRI/ACCESS-TEST#56
Note
The linked opened PRs are for
access-test-configs, which is a repository that isn't yet setup for actual QA/repro tests, which is why they failed. This commands scope ends at the point where a PR is opened in a configs repository (and optionally runs!test reproin a configs repo.Unit Tests were done via
pytestKnown Limitations
!update-configsis run twice on the same prerelease build, it will throw an error about a PR already existing for that configuration branch. In future, we will add commits to the existing PR instead.writepermissions on the MDR, they will be able to do!update-configsand by proxy open auto PRs into the linked configs repository. More testing needs to be done with this.MDR Requirements for Merging
secrets.CONFIGS_REPO_TOKENthat is passed into the entrypointsecrets.configs-repo-token. This token must havepull-requests:write,contents:writefor all configs repositories defined inconfig/auto-configs-pr.json. Speaking of...config/auto-configs-pr.jsonfile in their MDR (either in the PR they want to use the command, or in the default branch. This contains configuration information for the command.