CPP-2313 Add proper support for custom jobs in CircleCI #753
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ivomurrell
force-pushed
the
generated-job-executors
branch
2 times, most recently
from
51fa35c to
3b2e670
Compare
ivomurrell
force-pushed
the
generated-job-executors
branch
from
3b2e670 to
31ec896
Compare
Our previous logic for guessing whether to prepend our Tool Kit orb namespace to a workflow job didn't account for approval jobs, which don't need to be declared in a `jobs` stanza ahead of time. Also add some comments justifying why we're inferring whether to prepend the Tool Kit namespace as the logic is getting increasingly complicated.
All jobs specified in a CircleCI config need to have an execution environment assigned to it. Let's use the default node executor for our generated jobs, including a matrix of the node executors if appropriate.
This increases the complexity of the workflow job logic even further so I've refactored the code significantly as part of this in an effort to make it more readable.
This means we can reuse the aggregate error formatting instead of rewriting another version. It also means that the details field of ToolKitErrors can be preserved.
ivomurrell
force-pushed
the
generated-job-executors
branch
from
31ec896 to
93d6d49
Compare
meel-io
approved these changes
Jan 23, 2025
| // value to if it's from the orb or if it's a custom job that needs to be | ||
| // run for multiple Node versions | ||
| prefixedName.startsWith('tool-kit/') || | ||
| (runsOnMultipleNodeVersions && customJob && (customJob.splitIntoMatrix ?? true)) |
There was a problem hiding this comment.
thought: I was a little confused about this logic.
So a custom job that runs on multiple node versions will split into a matrix unless we opt out in the job definition or the workflow job definition? Opting out only at the workflow level will still set one executor.
Opting out at the job definition level won't set any executors because it should have been added at the job level by generateJob. Is that right?
If that's so, maybe expanding a bit more on why we return none for custom jobs would be useful.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Our initial 4.0 release of Tool Kit only contained minimal support for defining custom jobs in CircleCI. They didn't define the executor the job would run in, resulting in CircleCI validation errors unless they were added explicitly in a
customblock, so even calling it 'minimal support' might be a stretch. This PR adds proper support for the execution property, as well as adding asplitIntoMatrixoption to jobs which works the same as it does for workflow jobs. Efforts have also been made to refactor the code so that it's easier to follow.This now allows us to generate a CircleCI config for next-preflight using Tool Kit config exclusively without manual intervention.
Closes #740, as this PR carries on from the changes made in that PR.
Checklist:
feat(circleci): add support for nightly workflows,fix: set Heroku app name for staging apps too