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

Commit 6c930bc

Browse files
authored
fix: Halt if npm install fails; switch to npm ci as well (#1107)
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
1 parent f95a039 commit 6c930bc

File tree

3 files changed

+16
-4
lines changed

3 files changed

+16
-4
lines changed

docs/workflow.rst

+1-1
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ Although several micro-frontends (MFEs) are built into devstack (the full list i
6161

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

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

microfrontend.yml

+14-2
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,22 @@
1-
# This file contains configuration common too all microfrontends
1+
# This file contains configuration common to all microfrontends
22

33
version: "2.1"
44

55
services:
66
microfrontend:
7-
command: bash -c 'npm install; while true; do npm start; sleep 2; done'
7+
# Use `npm ci` rather than `npm install` for a few reasons:
8+
#
9+
# - Repeatability: Respect the currently checked out package
10+
# versions rather than upgrading when package.json and
11+
# package-lock.json don't match. (Two people using this at
12+
# different times on the same commit should get the same
13+
# results.)
14+
# - Immutability: Don't change the repo's working directory
15+
# unexpectedly when there's a lock mismatch.
16+
#
17+
# Fail fast if package install fails to avoid mysterious
18+
# errors later.
19+
command: bash -c 'npm ci || exit 1; while true; do npm start; sleep 2; done'
820
stdin_open: true
921
tty: true
1022
image: node:16

provision-insights.sh

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ echo -e "${GREEN}Installing requirements for ${name}...${NC}"
1313
docker-compose exec -T ${name} bash -e -c 'source /edx/app/insights/insights_env && cd /edx/app/insights/insights && make develop' -- ${name}
1414

1515
# # Install Insights npm dependencies
16-
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'
16+
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'
1717

1818
echo -e "${GREEN}Running migrations for ${name}...${NC}"
1919
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}

0 commit comments

Comments
 (0)