Typos, win compatibility, and other small fixes #426
Conversation
Not found on other parts of the project. Was failing due to service "/bin/bash" not found or something like that.
I don't know why this use of single quotes was having issues, but cdk was parsing it wrong and not removing the last single quote. Might be something specific to windows, but changing it solved the issue.
SSM editor was setting the variables on "env-name-qa-", as the service name is in $1
Otherwise `pnmp saas workers build` failed with `Error: Cannot find module '/app/packages/workers/node_modules/serverless/bin/serverless.js'` 😕
At least on Windows there seems to be issues with the permissions of these files. I had to run `git update-index --add --chmod=+x .\build_static.sh .\install_localstack_fixtures.sh .\run.sh .\run_local.sh .\run_migrations.sh .\run_tests.sh` to make it work, but I see chmod being used on the Worker's dockerfile so why not.
| @@ -2,7 +2,7 @@ | |||
|
|
|||
| set -e | |||
|
|
|||
| FULL_SERVICE_NAME="env-${PROJECT_NAME}-${ENV_STAGE}-$2"; | |||
| FULL_SERVICE_NAME="env-${PROJECT_NAME}-${ENV_STAGE}-$1"; | |||
There was a problem hiding this comment.
This change is breaking the ssm-editor by placing the script name instead of the given service name. It leads to setting the invalid path in the SSM Parameter Store, for example: /env-sbtest-qa-/scripts/run.sh/test
There was a problem hiding this comment.
Interestingly, I had a different experience and was only able to get the right paths on the SSM parameter store with $1. But some user in discord checked out my branch and had the opposite issue and had to revert to $2. So please leave it as $2 for now and when I test the deploy again I can look further into the issue.
|
|
||
| COPY nx.json tsconfig* jest* babel* .eslintrc* .prettier* .eslintrc* $APP_PATH/ | ||
| COPY $SRC_CORE_PATH $DEST_CORE_PATH/ | ||
| COPY $SRC_WORKERS_PATH $DEST_WORKERS_PATH/ | ||
| RUN chmod +x $DEST_WORKERS_PATH/scripts/*.sh | ||
|
|
||
| RUN pnpm install --include-workspace-root --frozen-lockfile --filter=workers... --filter=cli... |
There was a problem hiding this comment.
Could you explain the reason of changing the order of commands? Basically in this way it will install all pnpm deps on every source code change (it will not use cache for this) and we should avoid this
There was a problem hiding this comment.
So in my experience, if I don't move it, it builds successfully but it has the wrong modules and can not find the module serverless. I thought it was because COPY $SRC_WORKERS_PATH $DEST_WORKERS_PATH/ overwrites the node_modules, but that's false because it's in .dockerignore. Regarding the cache, yeah it's bad from a docker pov but I belive it gets cached by nx instead and the docker build command doesn't get triggered. Which also means you also need to pnpm nx clear-cache in between tests.
There was a problem hiding this comment.
I was able to reproduce right now.
- Revert position
- pnpm nx clear-cache
- pnpm nx run workers:compose-build-image
- pnpm saas up
- docker logs saas_{PROJECT_NAME}-workers-1
S C:\Users\me\saas_remote> docker logs saas_remote-workers-1
> nx run workers:serve
Local trigger server listening at http://0.0.0.0:3005
stdout:
> workers@2.3.0 sls /app/packages/workers
> sls "invoke" "local" "-f" "SynchronizeContentfulContent" "-d" "{\"source\":\"backend.contentfulSync\",\"detail-type\":\"complete\",\"detail\":{\"id\":\"d76db14f253e4586a016f878cb75a396\",\"type\":\"complete\"}}"
stderr: node:internal/modules/cjs/loader:1080
throw err;
^
Error: Cannot find module '/app/packages/workers/node_modules/serverless/bin/serverless.js'
at Module._resolveFilename (node:internal/modules/cjs/loader:1077:15)
at Module._load (node:internal/modules/cjs/loader:922:27)
at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
at node:internal/main/run_main_module:23:47 {
code: 'MODULE_NOT_FOUND',
requireStack: []
}
Node.js v18.17.1
stdout: ELIFECYCLE Command failed with exit code 1.
child process exited with code 1
| @@ -33,6 +33,7 @@ WORKDIR /app | |||
|
|
|||
| COPY . /app/ | |||
|
|
|||
| RUN chmod +x /app/scripts/runtime/*.sh | |||
There was a problem hiding this comment.
I think so as windows might loose the executable permission (discussed on discord). It is also done in the workers/Dockerfile so it's not that alien.
Please check if the PR fulfills these requirements
What kind of change does this PR introduce?
Many different changes that were necessary to follow the aws deploy documentation. Out of the box each of those was a blocker on Windows. I haven't been able to test on other platforms, but was able to reproduce each of them and confirm my changes fixed the issues.
What is the current behavior?
Many bugs and troubleshooting needed to follow the instructions out of the box... Not sure if it's because of windows, or lack of testing on recent changes.
What is the new behavior?
It just works, but there are still issues with the deploy of the workers and ecs, see linked issue, where again I don't know if they stem from OS or are originally there.
Does this PR introduce a breaking change?
If other people did not find this issues before and they stem from my OS, then for sure changes such as $2 -> $1 are going to break previously working behavior. Even though I would suspect it's broken for everyone, I would suggest someone tries a fresh setup and deploy of
qaas per the instructions with this changes on posix.Other information: