[FIX] Repair local Jupyter Conda permissions - #492
KilianTrunk wants to merge 4 commits into
Conversation
|
Thank you for this. I will take a look at this asap. |
| plotly | ||
|
|
||
| # LlamaIndex readers (hard imports in lexical-graph source) | ||
| USER root |
There was a problem hiding this comment.
is this necessary? I didn't think pip install required root access. We should keep the USER root call before the apt-get calls instead.
There was a problem hiding this comment.
I think this might make more sense in the follow-up PR when we do other installations. I just quite understand the problem with the current build.
There was a problem hiding this comment.
It's necessary, I can reproduce the failure from current main branch and it fails with:
ERROR: Could not install packages due to an OSError: [Errno 13] Permission denied: '/opt/conda/lib/python3.11/site-packages/backports/zstd'
Yeah pip install itself does not need the root, but the problem is that the earlier root-level Conda/pip step leaves files under /opt/conda that jovyan can't update.
The first version of this PR worked around that by installing the readers as root and repairing the permissions afterwards however I now changed it so that the permissions are repaired immediately after the root-level Conda/pip step instead.
| @@ -1,4 +1,4 @@ | |||
| name: local-dev | |||
| name: graphrag-toolkit-rdf-dev | |||
There was a problem hiding this comment.
this can cause migration issues. Can you pin the names of the volumes too?
Update lines 58...:
volumes:
# Pin explicit volume names so a Compose project rename does not orphan
# existing data. With an explicit name Compose uses it verbatim (no project
# prefix), so these always map to the same on-disk volumes regardless of the
# top-level `name:`.
neo4j_local_data_dev:
name: local-dev_neo4j_local_data_dev
neo4j_local_logs_dev:
name: local-dev_neo4j_local_logs_dev
pgvector_local_data_dev:
name: local-dev_pgvector_local_data_dev
There was a problem hiding this comment.
Hi Andrew, I have added the names for all three volumes and also I tested the rename path with an existing volume from the old project, and the renamed Compose project mounted the same volume instead of creating a new empty one 🙌
mykola-pereyma
left a comment
There was a problem hiding this comment.
The fix-permissions "${CONDA_DIR}" change and the volume pinning look good. One request on the Compose project rename — see the inline note on the name: line.
| @@ -1,4 +1,4 @@ | |||
| name: local-dev | |||
| name: graphrag-toolkit-rdf-dev | |||
There was a problem hiding this comment.
graphrag-toolkit-rdf-dev is out of step here — this stack runs neo4j-local + pgvector-local (not an RDF store), the SPARQL package that motivates "rdf" isn't in main yet, and it breaks the existing naming set (local-standard, local-dev, hybrid-standard, hybrid-dev).
Could we keep this PR as the permissions chore and leave the project as local-dev? The rdf naming would then land with the SPARQL store PR, where it matches reality.
There was a problem hiding this comment.
Done! Thank you for your time!
Co-authored-by: Matic <majericmatic@gmail.com> Co-authored-by: Tom Soru <tom@tommaso-soru.it>
bfe6860 to
605f091
Compare
Description
This PR isolates the local Jupyter image permission fix requested during the review of #405.
After the root-level Conda and pip installation step, it repairs the Conda installation permissions before the image switches back to the
jovyanruntime user. This prevents later package installation and notebook development workflows from failing with permission errors under/opt/conda.This PR contains no Compose project rename, volume migration, graph-operation changes, SPARQL implementation, documentation, notebooks,
.dockerignore, orDockerfile.hubchanges.Changes
fix-permissions "${CONDA_DIR}"after the root-level Conda and pip setup.local-dev).Problem
The image performs Conda and pip installation as
root, then runs asjovyan. Without repairing the Conda installation permissions, later package installation can fail with permission errors under/opt/conda.Compatibility
No Docker Compose project rename or volume migration is included. Existing
local-devnetworks, volumes, container names, and host ports remain unchanged.Testing
pytest) — not run because no Python source changeddocker compose -f examples/lexical-graph-local-dev/docker/docker-compose-dev.yml config --quietgit diff --check: passedChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.