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.

Sign up for GitHub

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 lambda-sqs-worker-cdk template #695

Merged
merged 6 commits into from
Nov 26, 2021
Merged

Fix lambda-sqs-worker-cdk template #695

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4bbd739
Update package.json
alazyzombie Nov 25, 2021
c4d5d65
fix lambda
Nov 25, 2021
911e95c
fix
Nov 26, 2021
99099b8
add changeset
samchungy Nov 26, 2021
8598084
You are a bold one
72636c Nov 26, 2021
248e13f
Merge branch 'master' into patch-1
72636c Nov 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions template/lambda-sqs-worker-cdk/.buildkite/pipeline.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,13 @@ steps:
label: 🧪 Test, Lint & Build
artifact_paths: lib/**/*
commands:
- yarn package
Copy link
Contributor Author

Choose a reason for hiding this comment

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

doesn't feel right to have to run yarn package here when it's already in Dockerfile

Copy link
Contributor

Choose a reason for hiding this comment

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

Does your test step require you to run yarn build beforehand?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I see what might be happening here. Do we need to mount lib/node_modules as a volume too? @72636c

Copy link
Contributor

@samchungy samchungy Nov 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Could you try something for me - could you add
- /workdir/lib/node_modules to the volumes section in your docker-compose file and undo all changes except the deploy script change in your package.json in your repo?

Copy link
Member

Choose a reason for hiding this comment

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

💡 That sounds right - it looks like lib/node_modules from yarn package is never mounted.

TBH I'm questioning why yarn package is even in the Dockerfile, it sounds like it belongs on the deploy step. I believe that was the rationale behind it being --offline, ideally it would be runnable without requiring access to the npm read token.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ooo that might be due to yarn cache clean afaik we need the cache to run any with --offline

Copy link
Member

Choose a reason for hiding this comment

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

Curious, is the command doing anything that cp -R doesn't?

Copy link
Contributor

@samchungy samchungy Nov 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Not sure aha, I think it may be possibly installing production node_modules in lib whilst leaving the Dev deps in tact? That's my theory. Running --production I think strips dev deps out yeah?

Copy link
Member

Choose a reason for hiding this comment

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

Ah of course, thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this worked, thanks!

- echo '--- yarn build'
- yarn build
- echo '+++ yarn test:ci'
- yarn test
- echo '--- yarn lint'
- yarn lint
- echo '--- yarn build'
- yarn build
env:
GET_GITHUB_TOKEN: please
plugins:
Expand Down
4 changes: 2 additions & 2 deletions template/lambda-sqs-worker-cdk/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@
"private": true,
"scripts": {
"build": "skuba build",
"deploy": "cdk deploy appStack --require-approval never --context stage=${ENVIRONMENT}",
"deploy": "cdk deploy appStack --require-approval never --context stage=${ENVIRONMENT} --progress events",
"format": "skuba format",
"lint": "skuba lint",
"package": "yarn install --ignore-optional --ignore-scripts --modules-folder ./lib/node_modules --non-interactive --offline --production",
"package": "yarn install --ignore-optional --ignore-scripts --modules-folder ./lib/node_modules --non-interactive --production",
"test": "skuba test",
"test:ci": "skuba test --coverage",
"test:watch": "skuba test --watch"
Expand Down