-
Notifications
You must be signed in to change notification settings - Fork 1
feat(config): add optional database connection CLI args #328
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
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.
Pull Request Overview
This PR adds optional command line arguments for database connection parameters (host, name, and username) to the CipherStash proxy. These CLI arguments provide an alternative to environment variables and configuration files for specifying database connection details when running the proxy binary directly.
- Adds new CLI arguments:
--db-host,--db-name, and--db-user - Refactors the
TandemConfig::buildmethod to accept anArgsstruct instead of just a file path - Updates all test cases to use the new
build_pathconvenience method
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/cipherstash-proxy/src/cli/mod.rs | Adds three new optional CLI arguments for database connection parameters |
| packages/cipherstash-proxy/src/config/tandem.rs | Refactors config building to accept Args struct and handle CLI overrides; adds build_path convenience method |
| packages/cipherstash-proxy/src/config/database.rs | Adds default username function with serde annotation |
| packages/cipherstash-proxy/src/proxy/mod.rs | Updates test to use new build_path method |
| packages/cipherstash-proxy/src/config/log.rs | Updates all tests to use new build_path method |
| packages/cipherstash-proxy-integration/src/migrate/mod.rs | Updates Args struct initialization with new CLI fields |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
freshtonic
left a comment
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.
A change with a clear benefit and it's fairly simple too. Nice one @coderdan.
auxesis
left a comment
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.
Love this, thanks for the PR @coderdan — big usability improvement.
Could you add something to docs/reference/index.md explaining how to start Proxy with these new command line options?
Looking forward to taking this for a spin. ❤️
0ce9519 to
1268ed2
Compare
Adds optional command line args to proxy for specifying the database, host and username.
If not provided, env and config is used as now.
If running in docker this has no effect but is very useful if running the binary directly.
Acknowledgment
By submitting this pull request, I confirm that CipherStash can use, modify, copy, and redistribute this contribution, under the terms of CipherStash's choice.