Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Feat: settings/configuration helper #144

Merged
merged 7 commits into from
Sep 12, 2022
Merged

Feat: settings/configuration helper #144

merged 7 commits into from
Sep 12, 2022

Conversation

thekaveman
Copy link
Member

@thekaveman thekaveman commented Sep 3, 2022 •
edited
Loading

Closes #142

Based on #140, will keep up to date with that branch and wait for it to be merged to main.

Introduces a new Configuration class in the settings.py module, with @property attributes for each of the settings VARIABLES. Property names are the lower_case version of their setting.

Use it like:

from eligibility_server.settings import Configuration

config = Configuration()

print(config.input_hash_algo)

Note: omitted the SQLALCHEMY settings as these are not used directly in the app code.

@thekaveman thekaveman added this to the Courtesy Cards milestone Sep 7, 2022
Base automatically changed from refactor/package to main September 7, 2022 21:56
@thekaveman
chore(settings): move default samples to default
4b7f2e0 
the sample settings are loaded by default when running the server locally/devcontainer
including in tests

this just moves them to be the default settings and leaves a note in the sample file
of how to override

helps writing tests that check defaults and change settings values
@thekaveman
feat(settings): define Configuration class
e64ebfe 
properties proxy to the current_app.config setting
@thekaveman
test(settings): test all Configuration helper attributes
84246af
@thekaveman thekaveman marked this pull request as ready for review September 7, 2022 22:25
@thekaveman thekaveman requested a review from a team as a code owner September 7, 2022 22:25
@thekaveman thekaveman self-assigned this Sep 7, 2022
@angela-tran angela-tran mentioned this pull request Sep 7, 2022
Merged
angela-tran
angela-tran approved these changes Sep 7, 2022
View reviewed changes
Copy link
Member

@angela-tran angela-tran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a big fan of the sample settings cleanup, the Configuration object, and the test_keypair cleanup that focuses more on reading the file. These changes look great 💯

@thekaveman thekaveman merged commit 3ffc4e0 into main Sep 12, 2022
@thekaveman thekaveman deleted the feat/configuration branch September 12, 2022 16:15
@angela-tran angela-tran mentioned this pull request Nov 14, 2022
Closed
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@angela-tran angela-tran angela-tran approved these changes

Assignees

@thekaveman thekaveman

Labels
None yet
Projects
None yet
Milestone
Courtesy Cards
Development

Successfully merging this pull request may close these issues.

Hide settings keys in an importable config object/module
2 participants
@thekaveman @angela-tran