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

Add docker volume for solr index; modify static files volume #842

Merged
merged 2 commits into from
Apr 16, 2024

Conversation

dchiller
Copy link
Collaborator

@dchiller dchiller commented Apr 3, 2024

This PR makes two changes to docker volumes:

  1. It adds a docker volume that contains the solr index files. This allows us to persist the index between container builds, rather than re-indexing every time the container is built.
  2. It reworks the static files volume that is used to share static files managed by django with the nginx container. Django-managed static files are now mapped to /code/static/django in the nginx container rather than /code/static. Node-managed static files are still mapped to /code/static. This means that any existing files in the volume persisted from build to build do no overwrite changes to static files introduced in a build of the nginx container. This fixes Remove cantus-min.js from static volume #841.

- Add solr volume containing solr index
- Change path of volume containing django-managed
static files
@ahankinson
Copy link
Member

I know I’m probably sounding like a broken record, but why not ditch docker and install Solr directly. Then you wouldn’t need to worry about persisting the data across container rebuilds…

@dchiller
Copy link
Collaborator Author

dchiller commented Apr 3, 2024

I know I’m probably sounding like a broken record, but why not ditch docker and install Solr directly.

I don't feel like I can make this decision re: Cantus Ultimus. It was "dockerized" two summers ago (I wasn't really involved in the "why", just the implementation of this decision, but I think the rationale was that all the other DDML projects used it?).

I'm not entirely opposed to the idea, especially now that I am more ansible and poetry comfortable, but yeah, doesn't feel like a decision I can take on my own.

The one thing that I still like about docker is that in development it separates some of the services that I use across projects: eg. I am literally running separate postgres and solr instances for Cantus Ultimus and CantusDB. Of course, I could run the same server locally and just have different databases and different cores, but there is something that feels nice about being able to tear down (for example) my postgres installation for Cantus Ultimus without affecting simultaneous work with CantusDB. I guess I could use a single docker container for development, but deploy directly to the vm (sans Docker). How do you think about this?

@fujinaga
Copy link
Member

fujinaga commented Apr 3, 2024 via email

Copy link

@jacobdgm jacobdgm left a comment

Choose a reason for hiding this comment

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

I think I understand the reasoning for creating a separate solr volume, but I'm confused why the URL path for existing django static files needs to change, and why the directory path for the django volume seems to be different in different places.

docker-compose.yaml Show resolved Hide resolved
@@ -132,7 +132,7 @@
# Static files (CSS, JavaScript, Images)
# https://docs.djangoproject.com/en/dev/howto/static-files/

STATIC_URL = "/static/"
STATIC_URL = "/static/django/"
Copy link

Choose a reason for hiding this comment

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

is this necessary? Does this mean that the paths for all static files will change from cantus.simssa.ca/static/filename.jpg -> cantus.simssa.ca/static/django/filename.jpg?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Does this mean that the paths for all static files will change from cantus.simssa.ca/static/filename.jpg -> cantus.simssa.ca/static/django/filename.jpg?

Yes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

is this necessary?

It's possible I could keep it the same and modify the nginx configuration.

Copy link

Choose a reason for hiding this comment

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

I guess it doesn't matter too much, but if we're serving a file called logo.png or something, I think the path /static/logo.png makes more sense - It's a static/ file called logo.png. Otherwise, /static/django/logo.png implies that it's a static/ file (good); not only is it a static file but within that it's a django/ (!?), which is called logo.png.

Thinking of writing up clear URIs probably matters less when we're serving images and bits of JS than when we're serving pages that represent things like manuscripts or chants, so if this would lead to significantly more complexity for developers, I think this is good as it is.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Having thought about this more, I agree. I don't like this approach -- better, I think, to have nginx handle the figuring out of where to look for things than exposing this (really rather arbitrary) division between "static files collected by django" and "static files we've decided to make part of the nginx container".

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

which, just to loop back to Andrew's point, is a fiction that is entirely Docker's fault

@jacobdgm jacobdgm self-requested a review April 4, 2024 17:28
@dchiller dchiller merged commit b85ab18 into DDMAL:main Apr 16, 2024
2 checks passed
@dchiller dchiller deleted the mod-static-volume branch April 16, 2024 15:46
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.

Remove cantus-min.js from static volume
4 participants