fix(cx-api): protocol version was wrong, which lead to a confusing error message #6033
Conversation
|
🚫 Fixes must contain a change to a test file |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
@nija-at Wrote:
Yes, basically if they upgrade their framework first, they will see a message instructing them to upgrade the CLI as well. @nija-at Wrote:
Only if the user upgrades the CLI first Remember that currently, users who have CLI This change makes it so that also users who will have CLI |
| @@ -75,7 +75,7 @@ export function upgradeAssemblyManifest(manifest: AssemblyManifest): AssemblyMan | |||
| // Backwards compatible changes to ContainerImageAssetMetadataEntry: | |||
| // * Make `imageNameParameter` optional (new apps do not require it anymore because container images go to a well-known repository) | |||
| // * Add optional `imageTag` to allow apps to specify exactly where to store the image (required if `imageNameParameter` is not defined) | |||
| manifest = justUpgradeVersion(manifest, '1.21.0'); | |||
| manifest = justUpgradeVersion(manifest, '1.23.0'); | |||
There was a problem hiding this comment.
Shouldn't this also include a section upgrading the version from 1.21.0 to 1.23.0?
We will be publishing Cloud Assemblies with version 1.21.0 with a few versions of the framework (1.22.0 at least).
There was a problem hiding this comment.
You are so right. Good catch! Fixed 👍
|
🚫 Fixes must contain a change to a test file |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
The ship has probably sailed with regards to this change. Closing this as we don't seem to need it. |
This PR changes the protocol version so that users start getting the correct mismatch error message.
Assuming this change will be released in
1.23.0.What are the affects of this change
Users having CLI version
<= 1.22.0who upgrade the framework, will receive the following error:Note that for users who have CLI <=
1.21.1and framework1.22.0, the message makes sense because they are currently in a broken state.For users who have CLI
1.22.0and framework1.22.0, the message actually breaks their setup, but at least the instruction is correct.Users who upgrade the CLI will not be affected.
While this change causes temporary breakage for some customers, it has the benefit of unblocking others. It also aligns our version to (kind of) what it should have been in the first place. Currently, the version doesn't point to the latest release, which is kind of bent.
What if we dont do this
Users who only upgrade their framework, will continue seeing the wrong error message, forever.
Our assumption is that users who see it, either upgrade the CLI on their own, or search for the error and wind up at our issue, instructing them to upgrade.
I don't like this because i feel we might be missing a class of users who are baffled and simply don't take any action to unblock themselves.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license