Skip to content
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(amplify): addBranch fails synth with "cannot find entry file..." #28772

Merged
merged 2 commits into from
Jan 19, 2024

Conversation

brettstack
Copy link
Contributor

@brettstack brettstack commented Jan 19, 2024

#28658 missed changing one of the paths (i.e it didn't remove the extra '..') and I was still getting the "cannot find entry file" error. I can confirm that my CDK stack works as expected after making the change locally.

This is my first CDK PR. I did the following:

yarn install
npx lerna run build --scope=@aws-cdk/aws-amplify-alpha
cd packages/@aws-cdk/aws-amplify-alpha
yarn build
yarn test
yarn integ-runner --update-on-failed
npx lerna run build --scope=aws-cdk-lib

Closes #28764


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

@aws-cdk-automation aws-cdk-automation requested a review from a team January 19, 2024 05:53
@github-actions github-actions bot added bug This issue is a bug. p2 beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK labels Jan 19, 2024
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

@brettstack
Copy link
Contributor Author

brettstack commented Jan 19, 2024

validate-pr is failing with "❌ Fixes must contain a change to an integration test file and the resulting snapshot."

Snaphots have been updated, but I don't think changes to existing integ tests are required. CC @kaizencc

Exemption Request Clarification Request

@aws-cdk-automation aws-cdk-automation added pr/reviewer-clarification-requested The contributor has requested clarification on feedback, a failing build, or a failing PR Linter run pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. labels Jan 19, 2024
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: 0c7b8fc
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Jan 19, 2024
@kaizencc kaizencc added pr-linter/exempt-integ-test The PR linter will not require integ test changes and removed pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. pr/reviewer-clarification-requested The contributor has requested clarification on feedback, a failing build, or a failing PR Linter run labels Jan 19, 2024
Copy link
Contributor

@kaizencc kaizencc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohhhhhh... crap. That is my bad. Thanks for the fix @brettstack

@aws-cdk-automation aws-cdk-automation dismissed their stale review January 19, 2024 14:09

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

@aws-cdk-automation aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Jan 19, 2024
Copy link
Contributor

mergify bot commented Jan 19, 2024

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit cb522bb into aws:main Jan 19, 2024
32 of 40 checks passed
mergify bot pushed a commit that referenced this pull request Feb 2, 2024
This is a follow up to #28658, #28772, and #28760. We had to fix multiple places where that file path extended beyond the package itself into other areas of the local repository (that would not be available after packaging). This caused myriad issues at synth time with `file not found` errors.

This PR introduces a linter rule with the following specifications:
- no inefficient paths, i.e. no going backwards multiple times. Ex. `path.join(__dirname, '..', 'folder', '..', 'another-folder')`. This should and can be easily simplified
- no paths that go backwards past a `package.json` file. This should catch the instances we faced next time.

The `yarn lint` command on `aws-cdk-lib` took 51.47s seconds without this new rule and 53.32s seconds with the rule enabled. The difference of ~2 seconds shouldn't be a hindrance in this case but I am happy to look for additional efficiencies in the rule I've written.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
SankyRed pushed a commit that referenced this pull request Feb 8, 2024
This is a follow up to #28658, #28772, and #28760. We had to fix multiple places where that file path extended beyond the package itself into other areas of the local repository (that would not be available after packaging). This caused myriad issues at synth time with `file not found` errors.

This PR introduces a linter rule with the following specifications:
- no inefficient paths, i.e. no going backwards multiple times. Ex. `path.join(__dirname, '..', 'folder', '..', 'another-folder')`. This should and can be easily simplified
- no paths that go backwards past a `package.json` file. This should catch the instances we faced next time.

The `yarn lint` command on `aws-cdk-lib` took 51.47s seconds without this new rule and 53.32s seconds with the rule enabled. The difference of ~2 seconds shouldn't be a hindrance in this case but I am happy to look for additional efficiencies in the rule I've written.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
TheRealAmazonKendra pushed a commit that referenced this pull request Feb 9, 2024
This is a follow up to #28658, #28772, and #28760. We had to fix multiple places where that file path extended beyond the package itself into other areas of the local repository (that would not be available after packaging). This caused myriad issues at synth time with `file not found` errors.

This PR introduces a linter rule with the following specifications:
- no inefficient paths, i.e. no going backwards multiple times. Ex. `path.join(__dirname, '..', 'folder', '..', 'another-folder')`. This should and can be easily simplified
- no paths that go backwards past a `package.json` file. This should catch the instances we faced next time.

The `yarn lint` command on `aws-cdk-lib` took 51.47s seconds without this new rule and 53.32s seconds with the rule enabled. The difference of ~2 seconds shouldn't be a hindrance in this case but I am happy to look for additional efficiencies in the rule I've written.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK bug This issue is a bug. p2 pr-linter/exempt-integ-test The PR linter will not require integ test changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Amplify: Cannot find entry file when calling addBranch with asset
3 participants