Skip to content

[CHORE] Fix local Jupyter permissions and rename the Compose project - #492

Open
KilianTrunk wants to merge 3 commits into
awslabs:mainfrom
Ortecha:chore/lexical-graph-dev-environment
Open

[CHORE] Fix local Jupyter permissions and rename the Compose project#492
KilianTrunk wants to merge 3 commits into
awslabs:mainfrom
Ortecha:chore/lexical-graph-dev-environment

Conversation

@KilianTrunk

Copy link
Copy Markdown

Description

This PR isolates the local lexical-graph development-environment changes requested during the review of #405.

It ensures that the Jupyter development image performs reader and system dependency installation with the required privileges, repairs permissions before returning to the notebook user, and gives the Docker Compose project a repository-specific name.

It contains no core graph-operation changes, SPARQL implementation, documentation, notebooks, .dockerignore, or Dockerfile.hub.

Changes

  • Rename the Docker Compose project from local-dev to graphrag-toolkit-rdf-dev.
  • Switch to root before installing the LlamaIndex reader dependencies and PyMuPDF.
  • Install the native build toolchain while running as root.
  • Run fix-permissions for the Conda installation and notebook-user home directory after root-level installation.
  • Return to the jovyan user for the final image runtime.
  • Keep the existing notebook command, services, ports, bind mounts, and dependency services unchanged.

Problem

The local Jupyter image performs package and system installation steps that require root privileges, but it must leave the Conda environment and notebook home directory writable by the final jovyan user.

The Compose project also used the generic name local-dev instead of a repository-specific name.

Related issue (if any): N/A

Compatibility note

Changing the Compose project name changes the generated Docker network and named-volume prefixes.

Existing resources created under the previous local-dev project name are not automatically reused under graphrag-toolkit-rdf-dev. Explicit container names and host ports are unchanged.

Testing

  • Unit tests added/updated (not applicable to this Docker-only change)
  • Integration tests added (not applicable)
  • Existing tests pass (pytest) — not run because no Python source changed
  • Tested manually as described below

Validation performed:

  • Docker Compose configuration validation: passed.
  • Dockerfile build check: passed with no warnings.
  • jupyter-local image build: passed.
  • Verified that the final runtime user is jovyan.
  • Verified that /opt/conda, /home/jovyan, and /tmp/pip-cache are writable by the runtime user.
  • Verified the following imports inside the built image:
    • PyMuPDF through fitz
    • llama_index.readers.web
    • llama_index.readers.file
    • llama_index.readers.github
    • llama_index.readers.json
    • llama_index.readers.s3
    • llama_index.readers.structured_data
  • git diff --check upstream/main..HEAD: no errors.
  • Merge-tree simulation against the current upstream/main: no conflicts.

Checklist

  • Code follows existing style and conventions
  • License headers present on new files (not applicable; no new files)
  • Documentation updated (not applicable)
  • No breaking changes, or changes are clearly documented

The Docker Compose project-name compatibility impact is documented above.


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Co-authored-by: Matic <majericmatic@gmail.com>
Co-authored-by: Tom Soru <tom@tommaso-soru.it>
@acarbonetto

Copy link
Copy Markdown
Collaborator

Thank you for this. I will take a look at this asap.

plotly

# LlamaIndex readers (hard imports in lexical-graph source)
USER root

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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 mykola-pereyma left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

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.

3 participants