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

Docker and backend changes to support prod-lite mode #877

Merged
merged 4 commits into from
Mar 28, 2024

Conversation

chriswilty
Copy link
Member

@chriswilty chriswilty commented Mar 25, 2024

Description

Enables running both API and UI through the Express server, in "production lite" mode.

From project root:

// Build UI, if not already built or if not up to date with latest changes:
npm run build

// Run UI and API together:
npm start

Resolves #867

Screenshots

Hosted on localhost:3000, and API reached on localhost:3000/api

image

Notes

  • Express server hosts UI as well as API, for running a built / bundled app
  • Running in development mode is unchanged: spin up API and UI separately
  • An additional Dockerfile is also provided, for running UI and API in same container
  • Original Dockerfile in backend directory remains, for running just the API in a container

Concerns

  • VITE_BACKEND_URL in frontend/.env.example has changed, so you will need to update your frontend/.env to match!

Checklist

Have you done the following?

  • Linked the relevant Issue
  • Added tests
  • Ensured the workflow steps are passing

Copy link
Contributor

@pmarsh-scottlogic pmarsh-scottlogic left a comment

Choose a reason for hiding this comment

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

All seems sensible enough (not that I can claim to fully understand it)! Ran things locally with no problem. Didn't test docker because I don't want to faff about installing. Approved, but there are a couple of comments.

.gitignore Outdated Show resolved Hide resolved
frontend/package.json Show resolved Hide resolved
@chriswilty
Copy link
Member Author

I nearly had a clean sweep of check failures 😓

On it....

@pmarsh-scottlogic
Copy link
Contributor

Good changes to the readme, I should add. Very clear

@chriswilty
Copy link
Member Author

@pmarsh-scottlogic Ok, this one should be good for a final sweep.

@chriswilty chriswilty force-pushed the 867-one-container-to-rule-them-all branch 2 times, most recently from 14729b4 to f4d8243 Compare March 28, 2024 08:57
Copy link
Contributor

@pmarsh-scottlogic pmarsh-scottlogic left a comment

Choose a reason for hiding this comment

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

looks good. Got 2 comments, both of which are not blocking me from approving

thumb
sketch

# End of https://www.toptal.com/developers/gitignore/api/react,node
Copy link
Contributor

Choose a reason for hiding this comment

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

this file wants a new line at the end of it

Copy link
Member Author

Choose a reason for hiding this comment

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

How strange, it has a newline in my local copy and it doesn't come up as a change.


export default express.Router().use(
express.static(
fileURLToPath(new URL('../../../frontend/dist', importMetaUrl())),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
fileURLToPath(new URL('../../../frontend/dist', importMetaUrl())),
fileURLToPath(new URL('@src/frontend/dist', importMetaUrl())),

does this work?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, because @src would be relative to backend, and we are needing the frontend build.

@chriswilty chriswilty force-pushed the 867-one-container-to-rule-them-all branch from f4d8243 to 512ef06 Compare March 28, 2024 10:58
@chriswilty chriswilty merged commit cacac12 into dev Mar 28, 2024
5 checks passed
@chriswilty chriswilty deleted the 867-one-container-to-rule-them-all branch March 28, 2024 11:00
chriswilty added a commit that referenced this pull request Apr 8, 2024
* Docker and backend changes to support prod-lite mode
* Bump app version to 1.0.0 across all package files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Simplified route to internal enterprise deployment
2 participants