Skip to content
This repository has been archived by the owner on Aug 1, 2024. It is now read-only.

Commit

Permalink
fix: Halt if npm install fails; switch to npm ci as well (#1107)
Browse files Browse the repository at this point in the history
We've seen a few cases where npm package installation fails, but the server
continues to try to start up, and then developers get very mysterious
errors that are hard to debug. Fail fast on MFE startup to avoid this.

Also switch to `npm ci` so that:

1. Two developers on the same repo commit get the same version
2. Repository working directories are not modified by just running an MFE

(Also, fix a typo.)

See edx/edx-arch-experiments#335
  • Loading branch information
timmc-edx authored Jun 27, 2023
1 parent f95a039 commit 6c930bc
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 4 deletions.
2 changes: 1 addition & 1 deletion docs/workflow.rst
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ Although several micro-frontends (MFEs) are built into devstack (the full list i

make dev.down.frontend-app-learning # Bring down the devstack version of the Learning MFE.
cd <path-to-frontend-app-learning> # Navigate to the Learning MFE's repository.
npm install && npm start # Install JS packages, and start the NPM devserver on your local host.
npm ci && npm start # Install JS packages, and start the NPM devserver on your local host.

Of course ``learning`` can be replaced with ``gradebook``, ``payment``, or another frontend-app name.

Expand Down
16 changes: 14 additions & 2 deletions microfrontend.yml
Original file line number Diff line number Diff line change
@@ -1,10 +1,22 @@
# This file contains configuration common too all microfrontends
# This file contains configuration common to all microfrontends

version: "2.1"

services:
microfrontend:
command: bash -c 'npm install; while true; do npm start; sleep 2; done'
# Use `npm ci` rather than `npm install` for a few reasons:
#
# - Repeatability: Respect the currently checked out package
# versions rather than upgrading when package.json and
# package-lock.json don't match. (Two people using this at
# different times on the same commit should get the same
# results.)
# - Immutability: Don't change the repo's working directory
# unexpectedly when there's a lock mismatch.
#
# Fail fast if package install fails to avoid mysterious
# errors later.
command: bash -c 'npm ci || exit 1; while true; do npm start; sleep 2; done'
stdin_open: true
tty: true
image: node:16
Expand Down
2 changes: 1 addition & 1 deletion provision-insights.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ echo -e "${GREEN}Installing requirements for ${name}...${NC}"
docker-compose exec -T ${name} bash -e -c 'source /edx/app/insights/insights_env && cd /edx/app/insights/insights && make develop' -- ${name}

# # Install Insights npm dependencies
docker-compose exec -T ${name} bash -e -c 'source /edx/app/insights/insights_env && cd /edx/app/insights/insights/ && npm install && ./npm-post-install.sh'
docker-compose exec -T ${name} bash -e -c 'source /edx/app/insights/insights_env && cd /edx/app/insights/insights/ && npm ci && ./npm-post-install.sh'

echo -e "${GREEN}Running migrations for ${name}...${NC}"
docker-compose exec -T ${name} bash -e -c 'source /edx/app/insights/insights_env && export DJANGO_SETTINGS_MODULE="analytics_dashboard.settings.devstack" && cd /edx/app/insights/insights && make migrate' -- ${name}
Expand Down

0 comments on commit 6c930bc

Please sign in to comment.