-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Use absolute qdrant directory in debian package #6716
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
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
@timvisee can you check out this pr? |
@@ -110,7 +110,7 @@ struct Args { | |||
/// Format: <config_file_path> | |||
/// | |||
/// Default path: config/config.yaml | |||
#[arg(long, value_name = "PATH")] | |||
#[arg(long, value_name = "PATH", env = "QDRANT_CONFIG_PATH")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we set the correct absolute path as default anywhere here?
I don't think we do. In which case Qdrant will not use the above config when installed by default, and it will not use the configured absolute paths.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, we set the default value in default config value settings.rs file. In linux after default installation user would have to set up the environment variable to use any other files, which in the debian package is already copied to etc/qdrant/config.yaml.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@timvisee can you let me know if you think anything should be changed. From what I have understood just setting an environment variables by using export in shell environment would do the work. There just adding an environment variables here does the job
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@timvisee can you please let me know your thoughts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In linux after default installation user would have to set up the environment variable to use any other files, which in the debian package is already copied to etc/qdrant/config.yaml.
I don't think this is the right approach. It would require additional configuration by the user, but we want to change default behavior here.
If we ship some default configuration and put it on disk, which we do. Qdrant should use the configuration at that path by default.
If we don't do that, the absolute paths this PR sets will not be used in deployments by default.
We should therefore make Qdrant look for the configuration at /etc/qdrant/config.yaml
by default, but only when packaged for Debian.
Thanks for working on this! Superseded by #6958 |
Closes: #3370
All Submissions:
dev
branch. Did you create your branch fromdev
?