Draft PRs disable schema and package projection checks

[CodeGat]

Closes #237

Background

Prereleases have evolved - sometimes they are used not to create a Release, but to have a sharable location to test out various builds. This means that the schema (and some of the checks) are unnessecarily restrictive. This PR removes schema checks and package projection checks for Draft PRs in Model Deployment Repositories.

Open Questions

Wouldn't mind @aidanheerdegen and @harshula s opinions on these questions:

• Say we have an environment that installs mom5@development with spack package hash abcdefg. If we create another environment that reuses this package and therefore it's hash, will we have a modulefile clash even with a modulefile name {name}/development-{hash:7 (resolves to mom5/development-abcdefg)?
  • And is this actually a problem? The packages are identical - it's not like the earlier case where we had {name}/development clashing the other, non-identical builds of mom5@development.

Testing

Using ACCESS-TEST with branch 237-skip-check-spack-yaml-check-on-draft_TEST (which is the PR branch with all access-nri workflow refs replaced with the above branch). Did this with sed -E 's|uses: access-nri/build-cd/(.+)@v4|uses: access-nri/build-cd/$1@237-skip-check-spack-yaml-check-on-draft_TEST|g' -i .github/workflows/*

See ACCESS-NRI/ACCESS-TEST#31 - this contains a run of tests that validate:

• Commit-invoked Draft PRs skip schema and package projection checks ✔️
• !redeploy-invoked Draft PRs skip schema and package projection checks ✔️
• Commit-invoked Non Draft PRs do NOT skip schema and package projection checks (and !bump continues to work) ✔️
• !redeploy-invoked Non Draft PRs do NOT skip schema and package projection checks ✔️

This verifies that this PR is working as expected.

[aidanheerdegen]

• And is this actually a problem?

What happens when a PR is closed or merged? Will the module file be deleted by one environment while it is still being used by another?

[CodeGat]

No, I've found out that modulefiles aren't handled by spack outside of creating them...gotta remove them ourselves!

[CodeGat]

I've verified that this PR works as it stands currently, so I'm setting it to Ready for Review - test details are in the PR description.

[aidanheerdegen]

What happens with !bump in a draft PR?

[CodeGat]

It'd still bump the version. It doesn't care what state the PR is in

[aidanheerdegen]

It'd still bump the version. It doesn't care what state the PR is in

Does that make sense for a draft PR which isn't checking the version stuff?

[aidanheerdegen]

will we have a modulefile clash even with a modulefile name? And is this actually a problem?

Yes, and given we're going to have to create logic to remove the module files it is a problem if we remove a modulefile that is being used by another PR build.

So we should probably prepend the PR namespace to the module file projection automagically so we can easily and safely clean them up when the pre-release is cleaned up.

[aidanheerdegen]

LGTM

[CodeGat]

It'd still bump the version. It doesn't care what state the PR is in

Does that make sense for a draft PR which isn't checking the version stuff?

I mean, not really. But it'd have to be a conscious choice to !bump the version in a Draft, which I think we should keep as functionality. Maybe someone wants to draft the PR as it's not ready to merge (yet) and wanted to bump the version anyway.

So we should probably prepend the PR namespace to the module file projection automagically so we can easily and safely clean them up when the pre-release is cleaned up.

I think this should be a separate issue, because it might have implications for users outside of this PR.