-
Notifications
You must be signed in to change notification settings - Fork 21
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.
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 Docker and upgrade Node #415
Changes from 29 commits
3c64705
7e66225
3bf256a
a2b6ca2
5afbab2
b28e273
53a4f3e
ebd1be5
dc866d2
596ebdd
1aa51c0
0e6ee65
2b1c742
ab76abb
0924268
08f986c
0634f9d
d8498ea
7fcee93
565a4c3
d40a7cc
9a400b8
4aea59e
ef25374
8ce35b8
8687fd7
d93e3a5
d74bd4d
27e15ca
f83e2f9
a2dd673
211eafc
d2841b3
daa6589
bf85545
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ jobs: | |
|
||
- name: "Install and Build 🔧" | ||
run: | | ||
corepack enable | ||
yarn | ||
yarn run storybook:build | ||
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using Corepack now, not yarnPath, so this file should be deleted |
This file was deleted.
trallard marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1 @@ | ||
nodeLinker: node-modules | ||
|
||
yarnPath: .yarn/releases/yarn-4.4.0.cjs | ||
trallard marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,14 +1,21 @@ | ||
FROM node:18.18-alpine3.18 | ||
# Keep this in sync with the Node version specified for the Conda dev environment (in environment_dev.yml) | ||
FROM node:22.8.0-alpine3.20 | ||
gabalafou marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
RUN corepack enable | ||
|
||
WORKDIR /usr/src/app | ||
|
||
COPY . . | ||
# Use the --immutable flag as a precaution. This guarantees that the Docker | ||
# build will fail if `yarn install` results in a different yarn.lock file than | ||
# the one checked into the repo. This could happen for example if the version of | ||
# Yarn used in one dev environment differs from the version used here. | ||
RUN --mount=type=cache,target=/root/.yarn YARN_CACHE_FOLDER=/root/.yarn \ | ||
gabalafou marked this conversation as resolved.
Show resolved
Hide resolved
|
||
yarn install --network-timeout 600000 && \ | ||
yarn install --immutable --network-timeout 600000 && \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added the --immutable flag here as a precaution. The Docker build will fail if the Yarn install process would result in a different yarn.lock file, which could happen for example if two different versions of Yarn are used. |
||
if [[ ! -f ".env" ]]; then mv .env.example .env; fi; | ||
|
||
|
||
|
||
EXPOSE 8000 | ||
|
||
CMD [ "yarn", "webpack-dev-server", "--port", "8000" ] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This command doesn't actually exist anymore but it was never updated. Probably because the image is never run with this default command but rather a command override is supplied by Docker Compose. |
||
CMD [ "yarn", "run", "start:ui" ] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,58 +25,7 @@ To learn how to use conda-store-ui alongside conda-store, please visit [the cond | |
|
||
## Development 👩🏻💻 | ||
|
||
To get started with conda-store-ui development, there are a couple of options, depending on the type of changes you are working on. | ||
This guide will help you to set up your local development environment. | ||
|
||
### Prerequisites 📋 | ||
|
||
Before setting up conda-store-ui, you must prepare your environment. | ||
|
||
We use [Docker Compose](https://docs.docker.com/compose/) to set up the infrastructure. Before starting ensure that you | ||
have Docker Compose installed. | ||
If you need to install Docker Compose, please see their [installation documentation](https://docs.docker.com/compose/install/) | ||
|
||
1. Clone the [conda-store-ui](https://github.com/conda-incubator/conda-store-ui.git) repository. | ||
2. Copy `.env.example` to `.env`. All default settings should work, but if you want to test against a different version | ||
of conda-store-server, you can specify if in the `.env` file by setting the `CONDA_STORE_SERVER_VERSION` variable to | ||
the desired version. | ||
Refer to the [Configuration documentation](https://conda.store/conda-store-ui/how-tos/configure-ui/) for more | ||
information on the `.env` file. | ||
|
||
### Local Development with conda-store-ui running in Docker 🐳 | ||
|
||
Running conda-store-ui in Docker is the most straightforward way to set up your local development environment. | ||
|
||
1. Run `yarn install`. This will download the needed JavaScript dependencies into a directory named `node_modules/`. | ||
This directory will later be copied into the `conda-store-ui` Docker container for use at runtime by the Conda Store | ||
UI app. | ||
2. Run `yarn run start:docker` to start the entire development stack. | ||
3. Open you local browser and go to [http://localhost:8000](http://localhost:8000) so see conda-store-ui. | ||
4. You can then log in using the default username of `username` and default password of `password`. | ||
|
||
Hot reloading is enabled, so when you make changes to source files, your browser will reload and reflect the changes. | ||
|
||
### Local Development without running conda-store-ui in Docker 💻 | ||
|
||
This setup still uses Docker for supporting services but runs conda-store-ui locally. | ||
|
||
#### Set up your environment | ||
|
||
This project uses [conda](https://conda.io) for package management. | ||
To set up conda, please see their [installation documentation](https://docs.conda.io/projects/conda/en/latest/user-guide/install/index.html). | ||
|
||
1. Change to the project root `cd conda-store-ui` | ||
2. From the project root create the conda environment `conda env create -f environment_dev.yml` | ||
3. Activate the development environment `conda activate cs-ui-dev-env` | ||
4. Install yarn dependencies `yarn install` | ||
|
||
#### Run the application | ||
|
||
1. Run `yarn run start` and wait for the application to finish starting up | ||
2. Open you local browser and go to [http://localhost:8000](http://localhost:8000) so see conda-store-ui. | ||
3. You can then log in using the default username of `username` and default password of `password`. | ||
|
||
Hot reloading is enabled, so when you make changes to source files, your browser will reload and reflect the changes. | ||
Please refer to the `conda-store` docs: [Local development setup for conda-store-ui codebase](https://conda.store/community/contribute/contribute/local-setup-ui). | ||
gabalafou marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
### Making a release 🚀 | ||
|
||
|
@@ -102,30 +51,25 @@ yarn test | |
|
||
Steps to install and set up: | ||
|
||
``` | ||
conda env create -f environment_dev.yml | ||
conda activate cs-ui-dev-env | ||
playwright install chromium | ||
cp .env.example .env | ||
corepack enable | ||
yarn install --immutable | ||
yarn build | ||
``` | ||
|
||
Line by line, here's what the commands above do: | ||
|
||
Comment on lines
+54
to
+65
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason why splitting commands then explanations? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe you can copy and paste the commands all at once |
||
1. Create Conda environment | ||
```sh | ||
conda env create -f environment_dev.yml | ||
``` | ||
2. Activate Conda environment | ||
```sh | ||
conda activate cs-ui-dev-env | ||
``` | ||
3. Install Playwright-usable browser | ||
```sh | ||
playwright install chromium | ||
``` | ||
4. Copy environment variables | ||
```sh | ||
cp .env.example .env | ||
``` | ||
5. Install JavaScript dependencies | ||
```sh | ||
yarn install --immutable | ||
``` | ||
6. Build app | ||
```sh | ||
yarn build | ||
``` | ||
5. Use Corepack to set Yarn to correct version | ||
6. Use Yarn to install JavaScript dependencies | ||
7. Build app | ||
|
||
To run the tests, you will need to run the following commands in two separate terminal windows | ||
or tabs: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -81,7 +81,7 @@ services: | |
|
||
conda-store-ui: | ||
build: . | ||
command: "yarn run start:ui --port 8000 --history-api-fallback" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can't actually pass these options this way. |
||
command: "yarn run start:ui" | ||
profiles: | ||
- local-dev | ||
ports: | ||
|
@@ -91,7 +91,8 @@ services: | |
condition: service_healthy | ||
platform: linux/amd64 | ||
volumes: | ||
- .:/usr/src/app | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was the real source of the "node_modules state file not found" error, not the Yarn version |
||
- ./src:/usr/src/app/src | ||
- ./style:/usr/src/app/style | ||
Comment on lines
+94
to
+95
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are really the only folders that can be watched for hot-reloading. Mounting everything was causing the .env file (which gets copied in the build step, see the Dockerfile) to go missing. |
||
healthcheck: | ||
test: | ||
["CMD", "curl", "--fail", "http://localhost:8000"] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -143,7 +143,7 @@ | |
"whatwg-fetch": "^3.6.2" | ||
}, | ||
"engines": { | ||
"node": ">=18.0.0" | ||
"node": ">=20.0.0" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So. My thinking here is that we pin both of the dev environments (Docker and Conda) to the same specific version (22.8.0), but we relax the constraint here. And to make things easier on ourselves, we drop official support for Node 18. My understanding is that changing the value in package.json will not prevent somebody from running the app with Node 18; it will just warn them. However:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My thinking behind pinning the dev versions to the same version is just to avoid the possibility of a node version related bug appearing in one environment but not the other and causing confusion There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My only hesitation about pinning on the conda environment, as listed before, is that with hard pins, one has to be quite disciplined about updating them regularly. Otherwise, one misses out on security updates and the like. I know this is true of the Docker image too, but unless it is infra/core system deps or to avoid hard incompatibilities then it is best to not add a hard pin to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah yes, what you write about security updates makes total sense to me. I'll relax the Node.js requirement in the Conda and Docker environments. |
||
}, | ||
"packageManager": "[email protected]" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not needed for Yarn classic (v1)