pin cdk versions for create amplify #2535
Conversation
🦋 Changeset detectedLatest commit: 94ff772 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| export const defaultDevPackages = [ | ||
| '@aws-amplify/backend', | ||
| '@aws-amplify/backend-cli', | ||
| '[email protected]', | ||
| '[email protected]', | ||
| 'constructs@^10.0.0', | ||
| 'typescript@^5.0.0', | ||
| 'tsx', | ||
| 'esbuild', | ||
| ]; | ||
|
|
||
| export const defaultProdPackages = ['aws-amplify']; |
There was a problem hiding this comment.
You're right this can be a plain JSON and will remove the need for regex
| // taken from lodash escapeRegExp and used to sanitize dep.name before using it in RegExp | ||
| // can be replaced by RegExp.escape once it is available | ||
| const safeDepName = dep.name.replace(/[\\^$.*+?()[\]{}|]/g, '\\$&'); | ||
| const depRegex = new RegExp(`'${safeDepName}.*'`); | ||
| defaultPackagesContent = defaultPackagesContent.replace( | ||
| depRegex, | ||
| `'${dep.name}@${dep.version}'` | ||
| ); |
There was a problem hiding this comment.
if we make default_packages.json we could get rid of regexes
.github/workflows/health_checks.yml
Outdated
| update_create_amplify_deps: | ||
| if: github.event_name == 'pull_request' | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
The automation for this is great idea.
In addition to this we should have:
- A check that asserts that some of
default_packages.tsare matching package.lock content. The check should block PR merges. This would be a gate to stop inconsistency, therefore this is a mandatory addition to have I would think. - Git hook to short circuit feedback loop to local machine that either does validation or upgrade or both. This is nice to have.
There was a problem hiding this comment.
- I agree there may be inconsistencies in the beginning, but won't the automation and this new check be redundant? Since each time package.lock is updated automation will update cdk versions in
default_packages.tsaccordingly - That's a great idea for some nice QOL, I'll look into adding this
There was a problem hiding this comment.
- If we were to craft extremely minimalistic automation, then the check would be automated and updates would be left manual. The goal of check is to prevent inconsistency, the goal of update is to reduce operational load on operators, those are different goals. But to your point about redundancy - if this job can be made is such a way that it
update_and_checks_create_amplify_despi.e. it either updates and early exists or asserts that versions are in sync as last step then it would fulfill the need for "check".
There was a problem hiding this comment.
Ok I see what you mean. What I currently have for update does have some checks but the idea of check being automated and updates left manual is growing on me and would be similar to check_api_changes.ts and the npm run update:api script.
Only problem I can think of is dependabot version updates but like with fixing any failing check we would go into the PR and make manual changes anyways (plus it'll only be for a subset of dependency updates).
There was a problem hiding this comment.
Added the automated check and the not-so-manual update that runs on git hook and can be run manually for tighter feedback loop.
|
|
||
| if (!ghContext.payload.pull_request) { | ||
| // event is not a pull request, return early | ||
| process.exit(); |
There was a problem hiding this comment.
can we use throw and return instead of process exists ?
.github/workflows/health_checks.yml
Outdated
| update_create_amplify_deps: | ||
| if: github.event_name == 'pull_request' | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
- If we were to craft extremely minimalistic automation, then the check would be automated and updates would be left manual. The goal of check is to prevent inconsistency, the goal of update is to reduce operational load on operators, those are different goals. But to your point about redundancy - if this job can be made is such a way that it
update_and_checks_create_amplify_despi.e. it either updates and early exists or asserts that versions are in sync as last step then it would fulfill the need for "check".
Problem
We are not pinning to versions of CDK being used in our repo for create amplify.
We also don't have an automated way to update these pinned versions.
Issue number, if available:
Changes
package-lock.jsonpackage-lock.json, then if there is any git diff it fails.Corresponding docs PR, if applicable:
Validation
Unit tests and #2534, which is a PR built on top of this one that has changes to CDK versions in
package-lock.json.Checklist
run-e2elabel set.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.