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

Serve static files at Django root #454

Merged
merged 2 commits into from
Apr 27, 2022
Merged

Conversation

rachelekm
Copy link
Contributor

Overview

We are currently using django-spa and a mastarm proxy on start to serve static files, however these static files were previously inaccessible to django-spa. This mounts the static assets created through mastarm to the django /static folder, allowing django-spa to find index.html on call to /.

Notes

  • The index.html file is not generated as part of the mastarm bundler and needed to be separately copied in, which is unideal. Ultimately, this serves as a temporary fix until we can move forward with a different bundler in Switch to a different bundler #376.

Testing Instructions

  • scripts/update
  • scripts/server
  • Navigate to http://localhost:8085/ and confirm static page loads

Resolves #424

Copy link
Contributor

@ktohalloran ktohalloran left a comment

Choose a reason for hiding this comment

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

I learned a lot about Docker volumes from reviewing this so thanks for adding me as a reviewer! As you can see from my comments, I wasn't able to get it to run as is; however, this was a tricky one trying to work around mastarm and the frontend and backend not sharing a root the same way they do in other projects, so good job isolating and addressing the issue on this one!

@@ -28,6 +28,7 @@ services:
volumes:
- ./src/backend:/usr/local/src/backend
- $HOME/.aws:/root/.aws:ro
- ./taui/assets:/usr/local/src/backend/static/assets
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
- ./taui/assets:/usr/local/src/backend/static/assets
- ./taui/assets:/usr/local/src/backend/static/assets
- ./taui/index.html:/usr/local/src/backend/static/index.html

Unfortunately, copying index.html as a command didn't work for me; however, mounting it as its own volume did. I'm not sure whether this is best practice but it makes sense to me that this file could persist across Docker containers like the rest of the stuff in taui/assets.

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'm thinking I must have already had the index.html file in my /static folder after trying to mount from before, so it was able to work for me locally but not actually in practice. This way makes sense to me and works well, so just pushed that change!

@@ -40,6 +41,7 @@ services:
- "--error-logfile=-"
- "--log-level=debug"
- "echo.wsgi"
- "cp ./taui/index.html /usr/local/src/backend/static"
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
- "cp ./taui/index.html /usr/local/src/backend/static"

If mounting index.html as its own volume works, this is no longer necessary.

@rachelekm rachelekm force-pushed the fix/rkm/django-static-files-fix branch from 8306279 to 507493c Compare April 19, 2022 12:50
@rachelekm rachelekm requested a review from ktohalloran April 25, 2022 21:08
Copy link
Contributor

@ktohalloran ktohalloran 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! 👍

@aaronxsu aaronxsu mentioned this pull request Apr 26, 2022
11 tasks
@rachelekm rachelekm merged commit c734392 into develop Apr 27, 2022
@rachelekm rachelekm deleted the fix/rkm/django-static-files-fix branch April 27, 2022 13:28
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.

Django root should serve static files
3 participants