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

Adding a dynamic way to include arbitrary dependencies of Wagtail #45

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

saevarom
Copy link
Collaborator

@saevarom saevarom commented Jun 20, 2022

Fixes #4

@zerolab
Copy link
Contributor

zerolab commented Jun 20, 2022

Hey @saevarom, thank you for this!
Tested locally and it is working mostly great.

A few things:

  1. I have django-modelcluster in libs. so had to docker compose exec web bash and pip install -e /code/libs/django-modelcluster
  2. Ditto, had to get into the web container and pip install -U django-taggit to make it work with the latest bakerydemo. Will try from scratch, rather than on top of existing install tomorrow morning
  3. Had to add the following:
diff --git a/.dockerignore b/.dockerignore
index 4b67e1f..44c04d0 100644
--- a/.dockerignore
+++ b/.dockerignore
@@ -8,3 +8,8 @@
 **/Vagrantfile
 **/.devcontainer
 **/.vscode
+**/.tox
+**/build/
+**/dist/
+**/venv/
+**/.venv/

in order to avoid copying a couple of GB of context because of package tox envs or venvs

@saevarom
Copy link
Collaborator Author

saevarom commented Jun 20, 2022

Thanks!

Regarding 1. and 2. Did you do a docker compose build web after switching to this branch?
I assumed that would rebuild the container with the contents of your /libs pip installed.

Regarding 3, we definitely should augment the .dockerignore with these lines to avoid copying all that data.

@zerolab
Copy link
Contributor

zerolab commented Jun 20, 2022

I did docker compose build. Checked again looking carefully at the very fast moving install lines and.. it looks like one of the libraries that I have has a dependency on an older Wagtail version, so that ends up installing that Wagtail version, thus modelcluster.

So this would happen even if one were to do this manually, or alter the Dockerfile to directly install the deps.

Augmenting .dockerignore is the only thing needed, imho

🌟

@zerolab
Copy link
Contributor

zerolab commented Oct 5, 2022

Just realised I also have

diff --git a/docker-compose.yml b/docker-compose.yml
index 63e6618..346c229 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -13,6 +13,7 @@ services:
     restart: "no"
     volumes:
       - ./wagtail:/code/wagtail:delegated,rw
+      - ./libs:/code/libs:delegated,rw
       - ./bakerydemo:/code/bakerydemo:delegated,rw
       - node_modules:/code/wagtail/node_modules/
     ports:

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.

Add Willow and django-modelcluster as locally editable dependencies
2 participants