-
Notifications
You must be signed in to change notification settings - Fork 46
Allow env variable substitution in CLI client properties #242
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 pull request adds environment variable substitution support to the CLI client properties file by replacing the standard Java Properties class with Apache Commons Configuration2's Configuration interface. This allows users to specify property values like password=${env:OIE_PASSWORD} in the configuration file, which will be resolved from environment variables at runtime.
Changes:
- Replaced
Propertieswith Apache Commons Configuration2'sConfigurationfor loading CLI configuration - Added new imports for Configuration2 classes
- Modified configuration loading logic to use
FileBasedConfigurationBuilderinstead of direct file reading
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Plan is to next test different scenarios with both correct and incorrect credentials: |
2dc36ff to
4b8c71d
Compare
…variables in config file. Signed-off-by: Ville Ranta-aho <ville@ville-pekka.com> Signed-off-by: Tony Germano <tony@germano.name>
4b8c71d to
da0757e
Compare
tonygermano
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.
I did the following:
- squashed all commits
- fixed whitespace issues
- rebased onto main
- removed some unnecessary imports
- reverted copilot suggestion and initialized config to empty BaseConfiguration
I tested all of your scenarios mentioned in #242 (comment)
I also tested with no config file
- no command line arguments (shows help)
- setting -a -u and -p on the command line (logs in if everything is valid)
- setting -a and -u, but not -p (shows help)
mgaffigan
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.
Did not test, but the changes look good.
Thank you @tonygermano ! |
Testing the functionality on Linux:
password=${env:OIE_PASSWORD}export OIE_PASSWORD=<password>~/engine/server/setup$ java -jar mirth-cli-launcher.jar -c ../../command/conf/mirth-cli-config.properties