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

Switch to an "app factory" #77

Closed
bjchambers opened this issue Feb 3, 2024 · 1 comment · Fixed by #78
Closed

Switch to an "app factory" #77

bjchambers opened this issue Feb 3, 2024 · 1 comment · Fixed by #78
Assignees
@bjchambers
Labels
refactor A code change that neither fixes a bug nor adds a feature

Comments

@bjchambers
Copy link
Contributor

This pattern will allow us to define a function that returns the application given a configuration. Then we can carry the configuration around as part of the application state, rather than relying on environment variables / global variables.
@bjchambers bjchambers added the refactor A code change that neither fixes a bug nor adds a feature label Feb 3, 2024
@bjchambers bjchambers self-assigned this Feb 3, 2024
@bjchambers
Copy link
Contributor Author

More importantly -- this issue caused me to lose a lot of time debugging when a test accidentally imported an HTTP method (from a module that imported the settings) rather than the method of the same name from the client. This caused the settings to be loaded from the environment too early. The app factory would make it more explicit.

bjchambers added a commit that referenced this issue Feb 4, 2024
@bjchambers
refactor: Switch to a factory method for the app
8b67257 
This allows the tests to inject a specific configuration directly,
rather than relying on having the environment set before the config
is loaded. It avoids errors like what took me a few hours to debug
when I imported the wrong thing in tests, causing the environment to
be read too early.

This closes #77.
@bjchambers bjchambers mentioned this issue Feb 4, 2024
Merged
bjchambers added a commit that referenced this issue Feb 8, 2024
@bjchambers
refactor: Switch to a factory method for the app (#78)
1ca7bfd 
* refactor: Switch to a factory method for the app

This allows the tests to inject a specific configuration directly,
rather than relying on having the environment set before the config
is loaded. It avoids errors like what took me a few hours to debug
when I imported the wrong thing in tests, causing the environment to
be read too early.

This closes #77.

* format and lint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bjchambers bjchambers

Labels
refactor A code change that neither fixes a bug nor adds a feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

refactor: Switch to a factory method for the app DewyKB/dewy
1 participant
@bjchambers