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.

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

Allow configuration parameters to be read from settings.py #198

Closed
phillipuniverse opened this issue Feb 9, 2022 · 5 comments · Fixed by #199
Closed

Allow configuration parameters to be read from settings.py #198

phillipuniverse opened this issue Feb 9, 2022 · 5 comments · Fixed by #199
Assignees

Comments

@phillipuniverse
Copy link
Contributor

For my use case we have some people that use Docker as a development environment and some that don't. This means the project root path could be /Users/phillip/projects/my_project (for me running on macOS) or /usr/src/my_project when inside of Docker. It would be nice if we could compute this config value through something like settings.py.

Perhaps there are other use cases for config to involve computation. Maybe there could be another option for reading configs and they could be considered in the following order:

  1. Read from a MIGRATION_LINTER section in settings.py
  2. Read from pyproject.toml/tox.ini/etc
  3. Read from command line

So command line arguments always take precedence over anything else.

Originally brought up in #167 (comment), copied here.

@David-Wobrock
Copy link
Collaborator

Hey,
Thanks for submitting the issue 👍

I'm wondering mainly about two things:
1/ the precedence between Django settings and config file.
We agree that the command line always overrides any of the above, but between settings and config file there is an open question.
My thoughts:

  1. Config files => the most static values, shared across environments
  2. Django settings => can be dynamic and therefore specific to an env
  3. Cmd line => the most manual, should always take precedence as stated above

2/ What API for the Django settings options? Two suggestions are:
a/ one constant for every option

MIGRATION_LINTER_OPTION_APP_LABEL = "my_app"
MIGRATION_LINTER_OPTION_PROJECT_ROOT_PATH = get_root_path()

b/ a dict with all options:

MIGRATION_LINTER_OPTIONS = {
    "app_label": "my_app",
    "project_root_path": get_root_path(),
}

My preference goes to option b/ in order to less pollute the constants and keep the values grouped together organically - but open to discussions here :)

Here is a PR with the above suggestions #199

@phillipuniverse
Copy link
Contributor Author

precedence between Django settings and config file

Great points here, I completely agree with you! Especially the “environment-specific” part.

API for Django setting options

I’m with you about polluting settings.pay with a bunch of first-class settings. But consider this - what does discovery look like? If you have them all on the parent level if I’m in Pycharm I believe I can type MIGRATION_LINTER in settings.py and then hit ctrl+space to pull up auto completion for all the settings. I think you lose that with a dict in a single setting.

On the other hand, the dict can be the exact same keys as the config file so it’s not too hard to look at the docs and copy/paste the right keys.

Finally, it might be possible to fix the autocompletion with a more first-class little dataclass object that represents the settings, and then you could type-hint the actual setting with that dataclass. Python 3.8+ has a nice TypedDict which could also work, maybe something to put in userland and not in this library.

So there’s the bike shed but what you’ve got in the PR is great!! Thanks so much @David-Wobrock!!

@David-Wobrock
Copy link
Collaborator

Thanks for the input Phillip!
Good point, but we might be overthinking/over-engineering this for now 😛 Let's move on with the currently implemented version, and we'll see in practice if a better design is needed :)

@phillipuniverse
Copy link
Contributor Author

@David-Wobrock good call! Thanks again for the addition!

Any thought towards what release this will land in and when?

@David-Wobrock
Copy link
Collaborator

I hope to tackle one or two open issues and then release 4.0.0 :) 🤞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants