feat: add config file with Settings class for database credentials - #113
Conversation
Implements #112 - Add config.py with Settings class using pydantic-settings - Add database_url property for MySQL connection string - Add unit tests for Settings class
There was a problem hiding this comment.
Pull request overview
This PR adds a new config.py module with a Settings class for managing database credentials via environment variables using pydantic-settings. However, this duplicates existing functionality already present in src/vetlog_buddy/shared/config.py, which has a more complete Settings class that includes the same database fields plus additional configuration (e.g., factor field) and proper .env file loading via a Config inner class.
Key Changes:
- Added
src/vetlog_buddy/config.pywith a Settings class reading db_host, db_name, db_user, db_password from environment variables - Added
database_urlproperty that constructs MySQL connection strings - Added unit tests in
tests/unit/test_config.pyusing unittest framework
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/vetlog_buddy/config.py | Introduces Settings class for database configuration, duplicating functionality from shared/config.py; lacks URL encoding for credentials and .env file loading configuration |
| tests/unit/test_config.py | Adds unit tests for Settings class using unittest (inconsistent with project's pytest convention); missing test coverage for error cases and special character handling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Hi @adityashirsatrao007 ; I appreciate that you sent a PR, however you have several comments from Copilot, please address those, so we can merge your PR.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
josdem
left a comment
There was a problem hiding this comment.
Only one more change to go and then we can merge. I appreciate your contriburion @adityashirsatrao007
|
@all-contributors please add @adityashirsatrao007 for code |
|
I've put up a pull request to add @adityashirsatrao007! 🎉 |
|
@copilot open a new pull request to apply changes based on the comments in this thread |
Done with the changes,you can check. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot open a new pull request to apply changes based on the comments in this thread |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@copilot open a new pull request to apply changes based on the comments in this thread |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@copilot open a new pull request to apply changes based on the comments in this thread |
jgafnea
left a comment
There was a problem hiding this comment.
Approved! Cached getter is a smart update for testability and cleaner DI.
josdem
left a comment
There was a problem hiding this comment.
It looks good to me, thanks for doing suggested changes @adityashirsatrao007 I appreciate it.
Summary
Adds a config module with a Settings class to read database credentials from environment variables.
Changes
Testing
\
python -m pytest tests/unit/test_config.py -v
\\
Closes #112