Skip to content

Commit

Permalink
Fix workflows and tests, update docs.
Browse files Browse the repository at this point in the history
  • Loading branch information
@Nospamas
Nospamas committed Dec 17, 2024
1 parent 26a0540 commit 05a7c39
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 188 deletions.
1 change: 1 addition & 0 deletions .github/workflows/docker-publish.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ jobs:
with:
username: ${{ secrets.pcicdevops_at_dockerhub_username }}
password: ${{ secrets.pcicdevops_at_dockerhub_password }}
dockerfile: docker/Dockerfile
repository: pcic/climate-explorer-frontend
tag_with_ref: true
build_args: REACT_APP_APP_VERSION=${{ env.REACT_APP_APP_VERSION }}
221 changes: 33 additions & 188 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,39 +22,20 @@ Note: Avoid `snap` packages. Experience to date suggests it does not result in s

## Configuration

### Environment variables
### Configuration variables

Main configuration of the Climate Explorer frontend is done via environment variables.
Configuration values are managed via a `config.js` file loaded into the app at runtime. Defaults (used for local
development) can be found in [public/config.js](./public/config.js). Adjust these as needed for your local environment
but the defaults _should_ work.

In a Create React App app, [environment variables are managed carefully](https://facebook.github.io/create-react-app/docs/adding-custom-environment-variables).
Therefore, most of the environment variables below begin with `REACT_APP_`, as required by CRA.

CRA also provides a convenient system for setting default values of environment variables
in various contexts (development, production, etc.).

Brief summary:

- `.env`: Global default settings
- `.env.development`: Development-specific settings (`npm start`)
- `.env.production`: Production-specific settings (`npm run build`)

For more details, see the
[CRA documentation](https://facebook.github.io/create-react-app/docs/adding-custom-environment-variables)).

Environment variables for configuring the app are:
Configuration variables for configuring the app are:

`PUBLIC_URL`

- Base **URL** for CE frontend app.
- For production, set this to the URL for CE configured in our proxy server.
- **WARNING**: The path component of this value **MUST** match `REACT_APP_CE_BASE_PATH` (see below).

`REACT_APP_CE_BASE_PATH`

- Base **path** of the URL for the CE frontend app;
- For production, set this to the path component of the URL for CE configured in our proxy server.
- **WARNING**: This **MUST** match the path component of `PUBLIC_URL` (see above).

`REACT_APP_APP_VERSION`

- Current version of the app.
Expand Down Expand Up @@ -102,9 +83,18 @@ Environment variables for configuring the app are:

- Path within the `public` folder of the external text file.

`NODE_ENV`
### Environment Variables:

Build configuration can still be managed via Environment variables however once built these will be unchangable
at runtime. Currently the following are defined:

- [**Automatically set; cannot be overridden manually.**](https://facebook.github.io/create-react-app/docs/adding-custom-environment-variables)
`PUBLIC_URL`: Defined as the local url when in development mode. Set to `%REPLACE_PUBLIC_URL%` during production
builds to allow for environmental configuration. See [entrypoint.sh](./docker/entrypoint.sh) for this mechanism.

`APP_VERSION`: set via the `generate-commitish.sh` script, used to bake the current version into the javascript files.

`NODE_ENV` [(unchangable)](https://facebook.github.io/create-react-app/docs/adding-custom-environment-variables): Set automatically to `production` when running `npm run build`, `development` otherwise. Allows for some
optimisation when executing production builds.

### Variable options

Expand All @@ -126,13 +116,9 @@ we "rebased" it on CRA.

### Installation

You **must** use a version of `npm` >= 5.5.1. This version of `npm` comes with `node` 9.2.0.
If you are using nvm, then run `nvm use 9.2.0` (or higher; ver 11.13 works fine too).

(`npm` 5.5.1 / `node` 9.2.0 is known to work; `npm` 3.5.2 / `node` 8.10.0 is known to fail to install certain required dependencies.
Intermediate versions may or may not work.)

Currently reccomended version is `node` 16.x and `npm` 8.x
This app has been tested against node 22, with its accompanying npm 10.9. These can be set up manually (we reccomend
the use of nvm or the `.devcontainer` specification can set up a container based development environment for you, including
executing an initial npm install for you.

With the appropriate versions of `node`/`npm` in use:

Expand Down Expand Up @@ -161,7 +147,7 @@ For building a production app, see below.
npm test
```

### Linting
### Linting / code style

Linting is handled by [Prettier](https://prettier.io/). Prettier can be run directly from the command
line or using the two aliased commands from the [package.json](package.json); `npm run lint` and `npm run format`.
Expand All @@ -180,45 +166,13 @@ The workflows setup in actions will automatically build, tag and publish to our

#### Configuration, environment variables, and Docker

It is best practice to configure a web app externally, at run-time, typically using environment variables for any
simple (atomic, e.g., string) configuration values.

Here we run into a problem introduced by CRA:
CRA injects environment variables only at _build time_, not at run time.
["The environment variables are embedded during the build time. Since Create React App produces a static
HTML/CSS/JS bundle, it can’t possibly read them at runtime."](https://facebook.github.io/create-react-app/docs/adding-custom-environment-variables).

We containerize our apps with Docker. A natural approach to deploying apps with Docker is to build the app as
part of the image build, and then just serve it from a container. However, because of CRA's build-time injection
of environment variables, this means that such Docker images cannot be configured at run-time, that is, when
a container is run. Only static, build-time environment variables are available to CRA apps inside such images.

It therefore takes some extra effort to inject run-time environment variables (or configuration generally) into
these Dockerized applications. There are two basic approaches:
Run-time configuration options are managed via a `config.js` file loaded into `index.html` when the app is loaded.
Defaults for these configuration options are kept in [public/config.js](./public/config.js), it should have
sensible defaults that allow local development. After deployment, this file can be replaced to adjust configuration
options to match the deployed environment. Within the container it is stored at `/app/config.js`.

1. Build the app, and inject run-time environment variables, as part of the image run (i.e., the command run
by the image includes building the app, which then has access to environment variables provided via the `docker run`
command). \* This is simple but it means that containers are slow to start up and contain a lot of infrastructure
(Node.js, etc.) needed to build the image. This isn't an issue for us, because we don't start many instances and
we don't do it often.

2. Fetch the environment variables (or a configuration file) from the server.
- This approach has several variants, which are outlined in this
[CRA issue](https://github.com/facebook/create-react-app/issues/2353).
- The way we load the external text and variable options files falls under into this category.

A key requirement is to be able to configure at run time the the URL at which the app is deployed.
CRA provides a (build-time) environment variable for this, `PUBLIC_URL`.
(Climate Explorer also, as a legacy from pre-CRA versions, uses an overlapping environment variable
`REACT_APP_CE_BASE_PATH`. See Configuration > Environment variables, above.)

Option 2 makes setting `PUBLIC_URL` _much_ harder to accomplish, and would require significant changes to the
codebase.

Option 1 makes setting `PUBLIC_URL` simple and requires almost no change to the codebase;
as noted we don't care about the cost of using such containers.

Therefore we have chosen option 1.
We no longer use environment variables for runtime configuration in order to avoid the build time associated with it.
They can be used to configure specific builds.

#### Routing and base path

Expand Down Expand Up @@ -246,9 +200,6 @@ Instead we must use CRA-provided build-time environment variable `PUBLIC_URL`.
to the url you provide (hostname included). This may be particularly useful
when using a CDN to host your application.

`PUBLIC_URL` serves much the same purpose as our custom env variable `REACT_APP_CE_BASE_PATH`.
This redundancy will be eliminated in a future verison of CE.

### Setup using Docker

We use Docker for production deployment.
Expand All @@ -257,92 +208,21 @@ It can also be useful in development; for example, to test a proposed volume mou

#### Build docker image manually

Until recently (roughly, Jan 2019), we were using Dockerhub automated builds
to build our images. Dockerhub recently changed their UI and in doing so broke
all the automated builds. For the moment we need to do manual builds.

Dockerhub images all had the name `pcic/climate-explorer-frontend`.

To distinguish our manually built images, we are omitting the `pcic/` portion
of the name and just using `climate-explorer-frontend`.

Build a docker image:

```bash
docker build -t climate-explorer-frontend \
--build-arg REACT_APP_APP_VERSION="$(./generate-commitish.sh)" .
```

Setting build arg `REACT_APP_APP_VERSION` as above is the most reliable
way to inject an accurate version into the final app. This value can be overridden
when the image is run, but it is not recommended, as it introduces the possibility
of error.

#### Tag docker image

Dockerhub automatically assigned the tag `latest` to the latest build.
That was convenient, but ...

For manual build procedures,
[tagging with `latest` is not considered a good idea](https://medium.com/@mccode/the-misunderstood-docker-tag-latest-af3babfd6375).
It is better (and easy and immediately obvious) to tag with version/release
numbers. In this example, we will tag with version 1.2.3.

1. Determine the recently built image's ID:

```bash
$ docker images
REPOSITORY TAG IMAGE ID CREATED SIZE
climate-explorer-frontend latest 14cb66d3d145 22 seconds ago 867MB

```

1. Tag the image:

```bash
Tag the image
$ docker tag 1040e7f07e5d docker-registry01.pcic.uvic.ca:5000/climate-explorer-frontend:1.2.3
```

#### Push docker image to PCIC docker registry

[PCIC maintains its own docker registry](https://pcic.uvic.ca/confluence/pages/viewpage.action?pageId=3506599). We place manual builds in this registry:

```bash
docker push docker-registry01.pcic.uvic.ca:5000/climate-explorer-frontend:1.2.3
```
A Makefile has been included for helping execute the creation of a docker image. This can be executed
via the command `make image`. It will build the javascript code into `build/` and create a docker image.

#### Run docker image

As described above, environment variables configure the app.
All are given standard development and production values in the files
`.env`, `.env.development`, and `.env.production`.

These can be overridden at run time by providing them in the `docker run` command (`-e` option).

In addition, we mount the configuration files as volumes in the container.
This enables us to update these files without rebuilding or redeploying the app.
See the section below for details.

Typical production run:

```bash
docker run --restart=unless-stopped -d
-e PUBLIC_URL=<deployment url, including base path>
-e REACT_APP_CE_BASE_PATH=<deployment base path>
-e <other env variable>=<value>
-p <external port>:8080
--name climate-explorer-frontend
- v /path/to/external/variable-options.yaml:/app/build/variable-options.yaml
- v /path/to/external/external-texts/default.yaml:/app/build/external-texts/default.yaml
climate-explorer-frontend:<tag>
```
The generated docker image can be tested via the `make up` command. This will start the container based
on the specification defined in the [docker/docker-compose.yaml](./docker/docker-compose.yaml) file.
Configuration options for its execution can be found in its accompanying [config.js](./docker/config.js),
these can be adjusted to suit your preferred configuration (such as adjusting the API source).

## Updating configuration files

Certain parts of Climate Explorer are configured in external configuration files.
These configuration files are stored in [the `public` folder](https://facebook.github.io/create-react-app/docs/using-the-public-folder).
The path to each configuration file inside this folder specified by an environment variable.
The path to each configuration file inside this folder specified by a value in the `config.js`.
Specifically:

| Configuration | Env variable | Default value |
Expand All @@ -360,16 +240,6 @@ in the docker container. (See section above.)
This external file can be modified, and the container restarted, to provide an updated version of the
variable options file without needing to modify source code, create a new release, or rebuild the image.

To change the configuration file without creating a new release of the app:

- Update the configuration file in the external file system.
- Restart the container (`docker restart climate-explorer-frontend`)

Alternatives:

- Stop the app and start it again with a different value for the associated environment variable,
and a corresponding volume mount for this new file.

To prevent tears, hair loss, and the cursing of your name by future developers (including yourself),
we **strongly recommend also updating** the source configuration files in the repo (in the `public` folder)
with any changes made, so that they are in fact propagated to later versions. "Hot updates" should not be stored
Expand All @@ -389,28 +259,3 @@ git commit -m"Bump to version x.x.x"
git tag -a -m"x.x.x" x.x.x
git push --follow-tags
```

## Code style standard compliance

We have nominally
[adopted](https://github.com/pacificclimate/climate-explorer-frontend/issues/138)
the default ESLint code style.
We aren't enforcing it right now, unfortunately.

Enforcement aside, we continue to commit code that is in violation of these
standards, which is undesirable for at least two reasons:

- Following a coding standard makes the code _much_ easier to read.
- For those of us with an IDE plugin that flags standards violations,
they appear _all over the place_, which is distracting to say the least.

The coding standard we adopted has a lot of rules, but the following are the
ones we are violating most. A small effort could radically reduce the number
of new violations we introduce. In approximate order of frequency of violation:

1. Limit line length to 80 characters.
1. Use single quotes for strings. (Double quotes are visually noisier.)
1. Place a space after a begin comment delimiter. (`// comment...`, not `//comment...`.)
1. Place a space between `if` and `for` and the opening parenthesis. (`if (cond)`, not `if(cond)`)
1. Declare variables with `const` or `let`, in that order of preference; avoid `var`. (`const` and `let` are scoped.)
- Use `for (const prop in obj)` and `for (const val of iterable)`, but `for (let i = 1; i < n; i++)`
3 changes: 3 additions & 0 deletions src/setupTests.js
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import "./core/lodash.mixins";

// include our default config properties
require("../public/config.js");

// configure({ adapter: new Adapter() });

// global.SVGPathElement = function () {};
Expand Down

0 comments on commit 05a7c39

Please sign in to comment.