Skip to content

Fix Undefined Volume Error by Using Absolute Path for Config File #893

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

Merged

Conversation

mmtmr
Copy link
Contributor

@mmtmr mmtmr commented Aug 20, 2024

Issue

Original Issue: Can not use --config-path when starting r2r with Docker #890

Description: When starting the r2r server with Docker using the --config-path option, the following error occurs:

Starting Docker Compose setup...
service "r2r" refers to undefined volume local_llm_neo4j_kg.toml: invalid compose project

This error indicates that Docker Compose is unable to locate the volume due to the config path being incorrectly interpreted.

Solution

To address this issue, I have updated the code to convert the config_path to its absolute path instead of just obtaining the basename. This change ensures that Docker Compose uses the correct absolute path for the volume, preventing the "Undefined Volume" error.

Code Changes

  • Updated Code: Changed the environment variable assignment to use os.path.abspath for config_path.

Before:

os.environ["CONFIG_PATH"] = (
    os.path.basename(config_path) if config_path else ""
)

After:

os.environ["CONFIG_PATH"] = (
    os.path.abspath(config_path) if config_path else ""
)

This modification ensures that the CONFIG_PATH environment variable contains the absolute path to the config file, allowing Docker Compose to correctly locate and mount the volume.

Testing

  1. Test Setup: Ensure that a local config file is placed in the current directory.
  2. Run Command: Start the r2r server with Docker using the command:
    r2r --config-path=local_llm_neo4j_kg.toml serve --docker
  3. Verification: Confirm that Docker Compose starts successfully without the "Undefined Volume" error.

🚀 This description was created by Ellipsis for commit a3b88c1

Summary:

Fixes Docker Compose volume error by using absolute path for CONFIG_PATH in r2r/cli/utils/docker_utils.py.

Key points:

  • Fixes issue Can not use config-path when start r2r with docker #890 related to Docker Compose volume error.
  • Updates r2r/cli/utils/docker_utils.py.
  • Modifies build_docker_command function.
  • Changes CONFIG_PATH to use os.path.abspath for absolute path.
  • Ensures Docker Compose locates config file correctly.

Generated with ❤️ by ellipsis.dev

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot 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 to me! Reviewed everything up to a3b88c1 in 13 seconds

More details
  • Looked at 14 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. r2r/cli/utils/docker_utils.py:353
  • Draft comment:
    Consider using os.path.abspath(config_path) to ensure the CONFIG_PATH is always an absolute path, similar to the change made in build_docker_command.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment is redundant because the code change already implements the suggestion. The comment does not provide any new information or actionable feedback.
    I might be overlooking the possibility that the comment is meant to reinforce the change, but the rules specify not to make purely informative comments.
    The rules clearly state not to make purely informative comments, so the comment should be removed as it does not add value.
    Remove the comment as it is redundant and does not provide any new or actionable information.

Workflow ID: wflow_mEuQLf8d4yOfOFqz


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@emrgnt-cmplxty emrgnt-cmplxty merged commit 210e112 into SciPhi-AI:main Aug 20, 2024
emrgnt-cmplxty added a commit that referenced this pull request Aug 23, 2024
* Fix argument order on ingest_files (#883)

* change get basename to get abspath of config (#893)

* Fix typos (#919)

* Fix typo in README (`Pyhon` -> `Python`)

* Fix incorrect setting name in error message

`collection` was used, but it should be `vecs_collection`.

* Fix type annotations to run with Python 3.9 (#943)

According to the `tool.poetry.dependencies.python` key of
`pyproject.toml`, the package supports `">=3.9,<3.13"`. However, when
running `r2r --help` with Python 3.9, I received the following error:

> `TypeError: unsupported operand type(s) for |: 'type' and 'NoneType'`

The problem was that one set of type annotations uses the `|` operator
instead of `typing.Union`, which was not supported until Python 3.10. I
switched to using the `typing` module to restore support for Python 3.9.

* Fix typo in variable name (#961)

* Add support for relative paths for the prompt directory in config (#958)

---------

Co-authored-by: Tim Kellogg <[email protected]>
Co-authored-by: Maxine Lai <[email protected]>
Co-authored-by: Alex Hedges <[email protected]>
Co-authored-by: Manuel R. Ciosici <[email protected]>
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.

2 participants