Fix static assets on staging #484

rachelekm · 2022-05-06T02:34:03Z

Overview

This allows for the frontend static assets bundled by mastarm to be available on staging as part of the django container image build.

Notes

Ideally there should be one fix for both deployed and local environments. I found that after changing the context to include /taui, copying assets via the Dockerfile, and taking the volume mounts out of the docker-compose yaml, it worked on deployed environments but no longer locally. I could make larger changes so that the assets could be built/bundled before the django container build in scripts/update (so that it better mirrored scripts/cibuild), but knowing this will eventually need to be rebased and altered with the new bundler PR Switch frontend app bundler #457, I opted to leave the volume/bind mounting in for local dev for now since it will be easier to take out.
On testing staging, the address field in user profile is not functioning. It looks like the access token we use for mapbox geocoding the address field in user profile is undefined and now we can't submit destinations. This has been addressed in issue Fix api access tokens on staging env #487

Testing Instructions

Delete any already built assets and their directory folder from your local environment: taui/assets and src/backend/static
optional Run docker system prune
Run scripts/update
Run scripts/server and test static files locally: http://localhost:8085/
Navigate to https://stg.echosearch.org/ and confirm frontend displays and you can reach https://stg.echosearch.org/profile

Resolves #471, #459

ddohler

Following the testing instructions didn't quite work for me because the Django Dockerfile assumes that the taui/assets/ folder isn't empty, but I think that should be relatively straightforward to fix.

ddohler · 2022-05-09T14:14:30Z

src/backend/Dockerfile

@@ -3,10 +3,13 @@ FROM quay.io/azavea/django:3.2-python3.10-slim
 RUN mkdir -p /usr/local/src/backend
 WORKDIR /usr/local/src/backend

-COPY requirements.txt /usr/local/src/backend/
+COPY /src/backend/requirements.txt /usr/local/src/backend/


Suggested change

COPY /src/backend/requirements.txt /usr/local/src/backend/

COPY ./src/backend/requirements.txt .

I think it's possible to use . for the destination because we've specified WORKDIR /usr/local/src/backend up above.

This worked! Will edit.

ddohler · 2022-05-09T14:24:35Z

scripts/update

@@ -26,6 +26,12 @@ if [ "${BASH_SOURCE[0]}" = "${0}" ]; then
        # Ensure container images are current
        docker-compose build


This command fails on a fresh build (when taui/assets is empty) because it tries to copy from taui/assets into the Django container, but fails because the folder is empty. I was able to resolve the issue by running the yarn build command below and then re-running scripts/update, so I think either we need to have two separate docker-compose build commands so that docker-compose build taui comes first and then docker-compose build database django comes after line 34, or we need to execute yarn build as part of the taui Dockerfile so that the static assets exist by the time the Django image is being built.

We talked about this in a separate discussion, but I set this up under the assumption that the directories taui/assets/ and src/backend/static/ themselves would not exist at the start of a fresh build—which is not always true, as evidenced here! I implemented the two separate builds as suggested so that regardless of a user's local environment the assets are made available to the Django container at volume mount regardless of whether the directory originally existed, didn't exist, or is empty.

ddohler · 2022-05-09T14:24:59Z

src/backend/Dockerfile

-COPY . /usr/local/src/backend
+COPY ./src/backend /usr/local/src/backend
+
+COPY ./taui/assets*/ /usr/local/src/backend/static/assets


Can this be just ./taui/assets/?

I tested this out and it does work without the asterisk now that we include yarn build as part of the taui docker build! But, I also had a scenario where I got rid of the taui/assets directory, ran scripts/update, and it skipped over the build since it was already cached and then failed because there was nothing there. I can't imagine this happening very often if ever in a local dev, but it offers a little extra resiliency when there may not be a taui/assets so I decided to leave it. Let me know if you think it will be cleaner to just take out!

Hmm, so interestingly enough I can't get the build to work (at all) with the asterisk there. Even if taui/assets/ exists and has files, I always get

ddohler@lima:~/Development/projects/echo-locator$ ls taui/assets/ config.json echo_combined_logo_fullcolor.svg echo_combined_logo_STACKED_fullcolor.svg index.js neighborhood_bounds.json neighborhoods.json config.json.map echo_combined_logo_fullcolor.svg.map echo_combined_logo_STACKED_fullcolor.svg.map index.js.map neighborhood_bounds.json.map neighborhoods.json.map ddohler@lima:~/Development/projects/echo-locator$ docker-compose build django Building django Sending build context to Docker daemon 1.2GB Step 1/9 : FROM quay.io/azavea/django:3.2-python3.10-slim ---> d158d6f22ceb Step 2/9 : RUN mkdir -p /usr/local/src/backend ---> Using cache ---> 77de371f9bb5 Step 3/9 : WORKDIR /usr/local/src/backend ---> Using cache ---> 7dbd4a9ca59c Step 4/9 : COPY ./src/backend/requirements.txt . ---> Using cache ---> 005fb203af59 Step 5/9 : RUN pip install --no-cache-dir -r requirements.txt ---> Using cache ---> d60532b194f2 Step 6/9 : COPY ./src/backend /usr/local/src/backend ---> Using cache ---> 42cc518b80e4 Step 7/9 : COPY ./taui/assets*/ /usr/local/src/backend/static/assets COPY failed: no source files were specified ERROR: Service 'django' failed to build : Build failed

I wasn't able to replicate this on my end but, as you suggested below, I think we can ditch the asterisk now that the taui yarn build command is in the Dockerfile vs directly in scripts/update. After making the change I was able to get this to succeed without the asterisk, so it's officially out and hopefully it doesn't cause trouble!

ddohler · 2022-05-09T14:28:31Z

scripts/cibuild

+        docker-compose \
+            -f docker-compose.yml \
+            run --rm --no-deps --entrypoint "bash -c" taui \
+            "yarn install && yarn build"


Is this necessary if it's also happening in scripts/update?

Ah, nope! Will take this out and leave the yarn install and build in scripts/update to cover asset bundling for both local and deployed setup.

Alas, I had to bring this back in. I thought I could still leave this out since yarn build was now included in the taui docker build, but it didn't build and bundle the assets as expected.

Ah okay, I think that having this in the taui Dockerfile vs. in scripts/update itself may be the source of the problem -- putting it in the Dockerfile means it'll be subject to Docker's rules about when to re-run a build step or when to use the cached layer, so sometimes it may get skipped. But since we're relying on this step to run in order to populate files on the host file system so that they can make it into another image, I think we want it to always run, no matter what. The only way to accomplish that is to put it in scripts/update itself, between the two docker-compose build calls, rather than in taui/Dockerfile. I think doing that should cause taui/assets to be filled with files in all cases when scripts/update is run, so then it should be possible to remove it from scripts/cibuild.

OH! Thank you for clarifying, that's really helpful. Ok I'll take that back out of the Dockerfile and into scripts/update and hopefully that does the trick!

This worked for me! This is back to being out of scripts/cibuild 🎉

rachelekm · 2022-05-10T15:25:12Z

Thanks so much for your edits and help better understanding the this build process. I implemented some of the changes you suggested/added some context in the replies from our separate discussion and hoping it should be more resilient now.

Local setup was previously dependent on there not being a /taui/assets/ dir, that way docker would ignore the command to copy on initial build in scripts/update when the assets had not been bundled yet. However, this isn't a very reliable setup since scripts/update could be called from a local environment and /taui/assets/ has potential to exist but be empty, which would throw an error on build. Instead, this waits to build the django container until after taui is built and bundled, so regardless of dev environment the build can succeed.

ddohler

Looks good! I didn't test on staging but building locally works successfully for me and I can open up the Django image without a volume mount and confirm that the taui assets show up inside.

rachelekm marked this pull request as ready for review May 6, 2022 14:18

rachelekm requested a review from ddohler May 6, 2022 14:18

rachelekm assigned ddohler May 6, 2022

ddohler suggested changes May 9, 2022

View reviewed changes

rachelekm force-pushed the test/rkm/build-static-assests-in-ci branch from 8868109 to a81bf24 Compare May 10, 2022 14:39

rachelekm requested a review from ddohler May 10, 2022 15:25

rachelekm added 5 commits May 12, 2022 14:58

Add static asset build to cibuild

ac6a92a

Adjust context dir for django container

a7a7e0a

Clean up unnecessary code

891f271

Move yarn build outside of Dockerfile

d92ab65

rachelekm force-pushed the test/rkm/build-static-assests-in-ci branch from a81bf24 to d92ab65 Compare May 12, 2022 19:05

ddohler approved these changes May 12, 2022

View reviewed changes

rachelekm merged commit 0b190a0 into develop May 12, 2022

rachelekm deleted the test/rkm/build-static-assests-in-ci branch May 12, 2022 21:12

aaronxsu mentioned this pull request May 13, 2022

Load dotenv file in CI/CD #500

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix static assets on staging #484

Fix static assets on staging #484

rachelekm commented May 6, 2022 •

edited

Loading

ddohler left a comment

ddohler May 9, 2022

rachelekm May 9, 2022

ddohler May 9, 2022

rachelekm May 10, 2022

ddohler May 9, 2022

rachelekm May 10, 2022

ddohler May 12, 2022 •

edited

Loading

rachelekm May 12, 2022

ddohler May 9, 2022

rachelekm May 9, 2022

rachelekm May 10, 2022

ddohler May 12, 2022 •

edited

Loading

rachelekm May 12, 2022

rachelekm May 12, 2022

rachelekm commented May 10, 2022

ddohler left a comment

	COPY /src/backend/requirements.txt /usr/local/src/backend/
	COPY ./src/backend/requirements.txt .

		@@ -26,6 +26,12 @@ if [ "${BASH_SOURCE[0]}" = "${0}" ]; then
		# Ensure container images are current
		docker-compose build

Fix static assets on staging #484

Fix static assets on staging #484

Conversation

rachelekm commented May 6, 2022 • edited Loading

Overview

Notes

Testing Instructions

ddohler left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ddohler May 12, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ddohler May 12, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rachelekm commented May 10, 2022

ddohler left a comment

Choose a reason for hiding this comment

rachelekm commented May 6, 2022 •

edited

Loading

ddohler May 12, 2022 •

edited

Loading

ddohler May 12, 2022 •

edited

Loading