Skip to content

Conversation

@fossamagna
Copy link
Contributor

@fossamagna fossamagna commented Mar 5, 2025

Description of changes

If Point in Time Recovery is enable whne RequestType is Create, ContinuousBackupsUnavailableException may occur.
When ContinuousBackupsUnavailableException is thrown, I changed to retry.

CDK / CloudFormation Parameters Changed

n/a

Issue #, if available

aws-amplify/amplify-backend#1654

Description of how you validated changes

  • Unit Test
  • Manual Test
    1. Build amplify-category-api
    git clone [email protected]:fossamagna/amplify-category-api.git
    git switch fix-amplify-backend-issue-1654
    yarn install
    yarn setup-dev
    1. Deploy test project
    git clone [email protected]:fossamagna/website.git
    pnpm install
    npx ampx sandbox
    1. Check if the CloudWatch logs show Backups are being enabled for the table in log streams for TableManagerCustomProvider.isComplete lambda function and if the deployment is successful.
      e.g: スクリーンショット 2025-03-05 12 10 27

Checklist

  • PR description included
  • yarn test passes
  • E2E test run linked
  • Tests are changed or added
  • Relevant documentation is changed or added (and PR referenced)
  • New AWS SDK calls or CloudFormation actions have been added to relevant test and service IAM policies
  • Any CDK or CloudFormation parameter changes are called out explicitly

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@fossamagna fossamagna marked this pull request as ready for review March 5, 2025 06:24
@fossamagna fossamagna requested a review from a team as a code owner March 5, 2025 06:24
@dpilch
Copy link
Contributor

dpilch commented Mar 5, 2025

The CI failed because the coverage decreased. I'm not sure why because you are adding the correct coverage.

Jest: "global" coverage threshold for branches (68%) not met: 67.26%
--
899 | Test Suites: 11 passed, 11 total
900 | Tests:       181 passed, 181 total
901 | Snapshots:   87 passed, 87 total
902 | Time:        52.333 s

Maybe because we are missing coverage on the else? https://github.com/aws-amplify/amplify-category-api/pull/3203/files#diff-f8f8c42c334f8453ee32eb354a7a49ad97ff4f900670f88ce74fecf93e253c48R436-R438

@fossamagna
Copy link
Contributor Author

@dpilch
Thank you for your suggestions.
I added some test cases to increase coverage.

@dpilch
Copy link
Contributor

dpilch commented Mar 6, 2025

@dpilch dpilch force-pushed the fix-amplify-backend-issue-1654 branch from 3988c56 to cdda4c2 Compare March 7, 2025 17:47
@dpilch dpilch merged commit 99fdf46 into aws-amplify:main Mar 19, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants