Skip to content
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

Merged
merged 35 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
3c64705
Fix Yarn and Docker
gabalafou Sep 4, 2024
7e66225
The actual changes needed
gabalafou Sep 4, 2024
3bf256a
revert yarn version change
gabalafou Sep 5, 2024
a2b6ca2
add comment to environment_dev.yml
gabalafou Sep 5, 2024
5afbab2
revert changes to yarn.lock
gabalafou Sep 5, 2024
b28e273
add --immutable flag to yarn install in Dockerfile
gabalafou Sep 5, 2024
53a4f3e
leave quotes
gabalafou Sep 5, 2024
ebd1be5
update GitHub workflows
gabalafou Sep 5, 2024
dc866d2
update README
gabalafou Sep 5, 2024
596ebdd
Merge branch 'main' into main
gabalafou Sep 5, 2024
1aa51c0
Update environment_dev.yml
gabalafou Sep 5, 2024
0e6ee65
Update Dockerfile
gabalafou Sep 5, 2024
2b1c742
Update Dockerfile
gabalafou Sep 5, 2024
ab76abb
Move corepack enable step up (to hopefully fix failing GitHub CI jobs)
gabalafou Sep 5, 2024
0924268
Update README.md
gabalafou Sep 6, 2024
08f986c
Update .github/workflows/release.yml
gabalafou Sep 6, 2024
0634f9d
Update .github/workflows/build.yml
gabalafou Sep 6, 2024
d8498ea
upgrade node
gabalafou Sep 6, 2024
7fcee93
do not test node 18 (drop support for it)
gabalafou Sep 6, 2024
565a4c3
put alpine back
gabalafou Sep 6, 2024
d40a7cc
make node more specific
gabalafou Sep 6, 2024
9a400b8
Merge remote-tracking branch 'upstream/main' into fix-docker
gabalafou Sep 9, 2024
4aea59e
move corepack enable command to correct place
gabalafou Sep 9, 2024
ef25374
correct list numbering in readme
gabalafou Sep 9, 2024
8ce35b8
Merge branch 'main' into main
gabalafou Sep 13, 2024
8687fd7
Apply suggestions from code review
gabalafou Sep 23, 2024
d93e3a5
Merge branch 'main' into fix-docker
gabalafou Sep 23, 2024
d74bd4d
Update environment_dev.yml
gabalafou Sep 23, 2024
27e15ca
Update Dockerfile
gabalafou Sep 23, 2024
f83e2f9
Update Dockerfile
gabalafou Sep 23, 2024
a2dd673
Update environment_dev.yml
gabalafou Sep 23, 2024
211eafc
Update Dockerfile
gabalafou Sep 23, 2024
d2841b3
correct docker image
gabalafou Sep 23, 2024
daa6589
Update environment_dev.yml
gabalafou Sep 23, 2024
bf85545
Update README.md
gabalafou Sep 23, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion .github/workflows/javascript-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,26 @@ jobs:

strategy:
matrix:
node-version: [18.x, 20.x, 22.x]
node-version: [20.x, 22.x]
# See supported Node.js release schedule at https://nodejs.org/en/about/releases/

steps:
- name: "Checkout repository 🛎"
uses: actions/checkout@v4

- name: "Set Yarn to correct version 📌"
run: corepack enable

- name: "Setup Node.js ${{ matrix.node-version }}"
uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node-version }}
cache: "yarn"
# https://github.com/actions/setup-node?tab=readme-ov-file#caching-global-packages-data

- name: "Install dependencies 📦"
run: yarn install --immutable
# https://yarnpkg.com/cli/install#options

- name: "Run tests 🧪"
run: yarn test
1 change: 1 addition & 0 deletions .github/workflows/pages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ jobs:

- name: "Install and Build 🔧"
run: |
corepack enable
yarn
yarn run storybook:build

Expand Down
8 changes: 7 additions & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,16 @@ jobs:
- name: "Checkout repository 🛎"
uses: actions/checkout@v4

- name: "Set Yarn to correct version 📌"
run: corepack enable

# Setup .npmrc file to publish to npm
- name: "Set up Node.js 🧶"
uses: actions/setup-node@v4
with:
node-version: "20.x"
node-version: "22.x"
registry-url: "https://registry.npmjs.org"
scope: "@conda-store-ui"
cache: "yarn"

- name: "Install dependencies 📦"
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ jobs:
run: |
cp .env.example .env

- name: "Set Yarn to correct version 📌"
run: corepack enable

- name: "Install Dependencies 📦"
run: |
sudo apt install wait-for-it -y
Expand Down
925 changes: 0 additions & 925 deletions .yarn/releases/yarn-4.4.0.cjs
Copy link
Contributor Author

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)

Copy link
Contributor Author

@gabalafou gabalafou Sep 5, 2024

Choose a reason for hiding this comment

The 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.

2 changes: 0 additions & 2 deletions .yarnrc.yml
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
9 changes: 6 additions & 3 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
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 . .
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 && \
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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" ]
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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" ]
88 changes: 16 additions & 72 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 🚀

Expand All @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why splitting commands then explanations?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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:
Expand Down
5 changes: 3 additions & 2 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ services:

conda-store-ui:
build: .
command: "yarn run start:ui --port 8000 --history-api-fallback"
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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:
Expand All @@ -91,7 +91,8 @@ services:
condition: service_healthy
platform: linux/amd64
volumes:
- .:/usr/src/app
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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"]
Expand Down
5 changes: 3 additions & 2 deletions environment_dev.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ channels:
- conda-forge
dependencies:
- python=3.10
- yarn>=4.4.0
- nodejs>=18.0
# Match the version of Node.js defined in the Dockerfile.
# Also, do not install Yarn separately, use Corepack (comes with Node.js).
- nodejs >=22.0
gabalafou marked this conversation as resolved.
Show resolved Hide resolved
- pytest
- pip
- pip:
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@
"whatwg-fetch": "^3.6.2"
},
"engines": {
"node": ">=18.0.0"
"node": ">=20.0.0"
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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:

  • when I ran yarn install after making this change, I did not get any warnings
  • when I tried it with npm install (v8.19.2), I did get a warning (but just a warning, it didn't exit with an error):
    npm WARN EBADENGINE Unsupported engine

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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

Copy link
Collaborator

Choose a reason for hiding this comment

The 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 major.minor

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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]"
}