fix: broken daemon build #170
Conversation
andreabadesso
force-pushed
the
fix/broken-daemon-build
branch
9 times, most recently
from
fe2078e to
6adf49a
Compare
andreabadesso
force-pushed
the
fix/broken-daemon-build
branch
from
ccb53ea to
3c23ce0
Compare
andreabadesso
force-pushed
the
fix/broken-daemon-build
branch
from
3c23ce0 to
77c7daf
Compare
| assertEnvVariablesExistence([ | ||
| 'NETWORK', | ||
| 'APPLICATION_NAME', | ||
| 'ACCOUNT_ID', | ||
| 'ALERT_MANAGER_REGION', | ||
| 'ALERT_MANAGER_TOPIC', | ||
| ]); | ||
|
|
There was a problem hiding this comment.
We have a config method that already checks for those variables and this check shouldn't be in a lib
| "references": [ | ||
| { "path": "../../node_modules/@wallet-service/common" } | ||
| ], |
There was a problem hiding this comment.
This makes tsconfig compile the common module tsconfig
There was a problem hiding this comment.
question(non-blocking): Should we add these instructions to a document on the repository itself?
There seem to be many of those small details that can break the production environment and delay future maintenance releases if left unchecked.
There was a problem hiding this comment.
I just found out that typescript implements its own JSON parser, which allows for comments in JSON
Added a comment in tsconfig.json in 536ee04
There was a problem hiding this comment.
Also created an issue so we document these details better
There was a problem hiding this comment.
Great! It will surely help with future maintenance
| COPY --from=builder /app/packages/daemon/dist . | ||
| COPY --from=builder /app/packages/daemon/node_modules ./node_modules | ||
| # This will remove all dev dependencies and install production deps only | ||
| RUN yarn workspaces focus -A --production |
There was a problem hiding this comment.
I added the -A flag because otherwise we wouldn't be able to use wallet-lib as a peer dependency and would have issues with bitcore-lib
This is not installing dependencies from the wallet-service lambdas as we are ignoring its package in .dockerignore
| "references": [ | ||
| { "path": "../../node_modules/@wallet-service/common" } | ||
| ], |
There was a problem hiding this comment.
question(non-blocking): Should we add these instructions to a document on the repository itself?
There seem to be many of those small details that can break the production environment and delay future maintenance releases if left unchecked.
Dockerfile
Outdated
| @@ -20,18 +19,11 @@ RUN corepack enable | |||
| RUN yarn set version 4.1.0 | |||
|
|
|||
| # This will install dependencies for the sync-daemon, devDependencies included: | |||
There was a problem hiding this comment.
Is this comment still valid?
There was a problem hiding this comment.
No, updated in 3e1a48f
Thanks
Motivation
The daemon was failing to load the common module during runtime because the typescript was not being transpiled to javascript. This was not an issue for the lambdas because the wallet-service uses webpack which has a typescript loader
Acceptance Criteria
Checklist
master, confirm this code is production-ready and can be included in future releases as soon as it gets merged