teacherc / spheri-app

Spheri is a simple web app that recommends a song based on your local weather
https://www.spheri.app
GNU General Public License v3.0
18 stars 3 forks source link

Constants and configuration #4

Closed BurningSquid closed 1 year ago

BurningSquid commented 2 years ago

https://github.com/teacherc/spheri-app/blob/fe4902431052b41d3914d44593e7c69c957a0dbe/code_dict.py#L1

I always make notes when I see stuff like this in people's project because I want to ask: is this configuration or are these constants. If constants they should go in a constants.py equivalent and use ALL_CAPS in naming as is convention. I encourage you to think about configuration though: are these values that the user would want to control? Are these values that should vary by country or by area? I always lean toward exposing more parameters to the top level configuration of the application as possible, can always just assign defaults or have advanced parameters that aren't normally visible.

BurningSquid commented 2 years ago

https://github.com/teacherc/spheri-app/blob/fe4902431052b41d3914d44593e7c69c957a0dbe/main.py#L80

Anytime you are using strings or "hardcoded" values I would ask the same questions as above.

teacherc commented 1 year ago

@BurningSquid I moved many constants into a constants.py file.

What about these variables? client_id = os.getenv('CLIENT_ID') client_secret = os.getenv('CLIENT_SECRET') message = f"{client_id}:{client_secret}" messageBytes = message.encode("ascii") base64Bytes = base64.b64encode(messageBytes) base64Message = base64Bytes.decode("ascii")

Are these configurations? If so, do I keep them where they are (in a spotify auth function) or move them and reference them in the function?

BurningSquid commented 1 year ago

Good question! Anything that gets values from environmental variables I would put either in constants or in it's own "settings.py" file if you have enough of them. In terms of the encoding for the spotify request, since that is purely in service of that one method you might leave it in there. If you find that you are doing multiple requests to spotify and need them anywhere else you can always pull them out.

teacherc commented 1 year ago

@BurningSquid Are client_id and client_secret constants (the rest being variables)?

BurningSquid commented 1 year ago

Right. Sorry didn't mean to not address that directly - they are technically system-level configuration not constants. Constants should be things that never vary at all - either by deployment endpoint, user, session, etc. Configuration is a bit of a vague term because it can fall into multiple different categories. For example a particular project can have session-level configuration (think settings for a particular user instance of the app), user-level (like user-specific settings like dark mode or other preferences), as well as system level (like authentication params for different deployments, resource limits for k8s, etc).

Quick edit: this isn't exhaustive, there can be many other layers of configuration that are needed depending on project architecture. For example, in the ML world there is configuration for training, feature engineering, etc etc

Does that help?

teacherc commented 1 year ago

@BurningSquid That makes a lot of sense, thanks!