-
Notifications
You must be signed in to change notification settings - Fork 824
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: block init if amplify app creation fails #10839
fix: block init if amplify app creation fails #10839
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #10839 +/- ##
=======================================
Coverage 47.43% 47.43%
=======================================
Files 673 673
Lines 33274 33275 +1
Branches 6724 6721 -3
=======================================
+ Hits 15782 15784 +2
Misses 15803 15803
+ Partials 1689 1688 -1
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
packages/amplify-provider-awscloudformation/src/amplify-service-manager.js
Outdated
Show resolved
Hide resolved
2e4935b
to
8ae1f65
Compare
8ae1f65
to
9612c67
Compare
packages/amplify-provider-awscloudformation/src/amplify-service-manager.js
Fixed
Show fixed
Hide fixed
packages/amplify-provider-awscloudformation/src/amplify-service-manager.js
Fixed
Show fixed
Hide fixed
packages/amplify-provider-awscloudformation/src/amplify-service-manager.js
Fixed
Show fixed
Hide fixed
packages/amplify-provider-awscloudformation/src/amplify-service-manager.js
Outdated
Show resolved
Hide resolved
packages/amplify-provider-awscloudformation/src/amplify-service-manager.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Please get clean e2e run before merging.
Could you split TS conversion into separate PR and get this also to |
Description of changes
Blocks
amplify init
if the Amplify App service limit has been reached. This prevents downstream errors and weird behavior where an Amplify App is expected to exist.Issue #, if available
Fixes #10838
Description of how you validated changes
Tested locally, added unit test, E2E running here: https://app.circleci.com/pipelines/github/aws-amplify/amplify-cli?branch=run-e2e/foyleef/app-limit-init-block
Checklist
yarn test
passesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.