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

Discussion: from_envvar's behavior #2368

Closed
tony opened this issue Jun 11, 2017 · 4 comments
Closed

Discussion: from_envvar's behavior #2368

tony opened this issue Jun 11, 2017 · 4 comments

Comments

@tony
Copy link
Contributor

@tony tony commented Jun 11, 2017

There is an issue I've been having for a few years regarding Flask and I failed to articulate it correctly. I find from_envvar's behavior unintuitive. Also, I haven't been able to find prior discussions mentioning it. Probably because of Chesterton's fence, or they feel they'd be told to work around it. Which only takes one line:

app.config.from_object(os.environ["app.config.dev"]), see Google results 😄

The first google result I see is people perplexed by from_envvar. Why? I'm not sure. For some reason, in recent years, Django (and frankly, Flask also) got devs accustomed to importing modules via strings.

Current behavior:

  • from_envvar forwards to from_pyfile, which strictly requires an absolute/relative file path (i.e. not a module string).

Expected behavior:

  • from_envvar should wrap from_object

    Why:

    • from_envvar only works with files

    • File from from_envvar are interpreted as modules anyway

    • from_envvar (from_pyfile) cannot point to classes

    • from_object supports string module/class paths (e.g. app.config.dev), which works with environmental variables

    • from_objectsupports both classes and modules. Especially important since the official documentation gives examples of using classes.

    • It's more customary for users invoking configuration by files to do so via CLI arguments.

    • A local, dev, staging, or production deployment in practice would opt to use environmental variables to point to a python object (module/class) via string.

      For instance: Let's say we're deploying to production with a uwsgi config. Since python production deployment is highly driven by packages, we've likely already done the work of figuring out our python environmental / package locations. It'd be more intuitive to point to a python object by string, rather than by file path.

  • Alternative: from_envvar should additionally wrap from_object for non-file paths.

  • Python version: 3.6
  • Flask version: 1.12
  • Werkzeug version: 0.12.2
@ThiefMaster

This comment has been minimized.

Copy link
Member

@ThiefMaster ThiefMaster commented Jun 11, 2017

IMO a config file does not belong the package directory, but your suggestion sounds a lot like you want the environment variable to specify an import name (such as myapp.config) instead of a path (e.g. /opt/myapp/etc/myapp.conf)'. I don't think this makes much sense.

Let's say I install my app in /opt/myapp. So I have a virtualenv in /opt/myapp/.venv and the myapp package ends up inside its site-packages directory when installed. However, does a config file belong in site-packages? IMO the answer here is a clear no. It should be in some other location. Could be /opt/myapp/myapp.conf or /opt/myapp/etc/myapp.conf or really anything else. It's a config file. Pretty much everything that lets the user override the default config takes a file path specifying where the config can be found. I don't think Flask (or any other framework) should go a different path here.

I know there are some example projects out there that have config/{defaults,dev,prod}.py inside the package but I think this is an awful idea. Except for default configs, configuration files for deployment do not belong into the package (or the repo containing the package). When publishing an application to PyPI (or even a local pypiserver in case it's not a public/open-source project), you might want to be able to install it on different environments which may require different configs - sure, these config files should probably be versioned somewhere. But I think deployment configs and the application codebase itself should remain separate.

@tony

This comment has been minimized.

Copy link
Contributor Author

@tony tony commented Jun 11, 2017

IMO a config file does not belong the package directory
However, does a config file belong in site-packages? IMO the answer here is a clear no.

It's a common pattern in Django, and even in the Flask community.

Example: https://github.com/rtfd/readthedocs.org/tree/master/readthedocs/settings

A check of github code search shows a decent share of Flask, >50%, use from_object:

A popular flask example, flask-overholt using a config in its own package and applying it in a factory.

I could probably give a lot more to support that module strings are commonplace.

I'm not saying that it's righteous or technically superior, but it's a de facto way of loading configs, especially in Flask.

Pretty much everything that lets the user override the default config takes a file path specifying where the config can be found. I don't think Flask (or any other framework) should go a different path here.

I'm trying to make sure I'm understanding that correctly.

Flask supports importing from objects. There's also examples in official documentation.

In addition, what if the configuration deliberately wants to use site-packages? Especially when they deploy? For instance, dj-database-url is imported in django settings. (I'm assuming they're using project's site-packages and not their system's.)

Even in situations where configurations are not stored in site-packages, people try to do add the directory to site-packages manually anyway: Here are a few cases of uwsgi scripts adding directories for settings by hand via sys.path.append and site.addsitedir.

@tony

This comment has been minimized.

Copy link
Contributor Author

@tony tony commented Jun 11, 2017

I know there are some example projects out there that have config/{defaults,dev,prod}.py inside the package but I think this is an awful idea. Except for default configs, configuration files for deployment do not belong into the package (or the repo containing the package). When publishing an application to PyPI (or even a local pypiserver in case it's not a public/open-source project), you might want to be able to install it on different environments which may require different configs - sure, these config files should probably be versioned somewhere. But I think deployment configs and the application codebase itself should remain separate.

Let's assume you're right. Keeping configs with special/sensitive stuff (like db credentials) there is a bad idea.

Flask has already been around and supported configs that are in classes. These can't be invoked via from_envvar. So are the people who went the class route for configs on their own?

Keep in mind, this discussion is not about removing from_pyfile. It's about from_envvar's default behavior. Primarily that it doesn't support from_object at all, and feels out of place with the rest of the config features. There's a lot more happening than just files.

Except for default configs, configuration files for deployment do not belong into the package (or the repo containing the package).

That makes sense.

When publishing an application to PyPI (or even a local pypiserver in case it's not a public/open-source project), you might want to be able to install it on different environments which may require different configs - sure, these config files should probably be versioned somewhere. But I think deployment configs and the application codebase itself should remain separate.

Let's assume that.

It doesn't rule out having settings added to site packages from outside the project's code being imported via a string (from_object). And by extension, from an environmental variable. I assume, some of the people using app.config.from_object(os.environ['foo']) are splitting things up securely. How? I don't know. It likely varies and since it's internal and special to their setup, they don't need to go into specifics publicly.

@davidism

This comment has been minimized.

Copy link
Member

@davidism davidism commented Jun 13, 2017

I'm happy with the app.config.from_object(os.environ['FLASK_CONFIG']) solution. As Thief says, from_envvar is intended for local one off files. These files are exec'd in the current env, so there's no reason they couldn't import other things from the env.

The next version will also allow FLASK_APP=app:factory('dev') to pass arguments to an app factory, which I think is the most common case for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.