Skip to content

Support overriding config options via environment variables #48

Closed

Description

@embray

I first wrote this as a comment in #35, but I'm creating a separate issue for this so it doesn't get forgotten. I will volunteer to implement it. This is slightly higher priority now that I'm getting somewhere with PyFITS implementation, and I would like to hook it into the eventual config system.

It would be good to have the ability to override any config options via environment variables. These environment variables would follow some standard naming convention like ASTROPY_SUB_PACKAGE_OPTION. There could also be some way to register shorter aliases for commonly used options, but I don't have any thoughts yet on the specifics of that.

In order to support these environment variables transparently, the easiest approach I see (which I usually end up doing anyways) is to provide a simple wrapper around ConfigObj objects. I like to use something simple like Configuration. This is a much simpler object than the ConfigObj so that fewer details of reading/writing the configuration are exposed to subpackage/affiliated package authors. It would still provide a dictionary-like interface and many of the methods of ConfigObj. Though rather than write() it has a save() method which I think is a little more obvious. If a user wants to update the configuration they simply call config.save(). This method would prevent saving any values that were overridden by environment variables.

Using a wrapper can have other eventual benefits, since it allows us to provide some abstractions around the configuration object that aren't specific to any one backend implementation (not that I would suggest we move away from ConfigObj or anything--it's just nice to have additional flexibility).

Activity

  1. ghost assigned on Nov 7, 2011
  2. embray commented on Dec 7, 2011

    @embray
    MemberAuthor

    I've started poking at this a bit in iguananaut/astropy/config-envvars, but hit a small snag. From my last commit message:

    When a config is written to a file I don't want it to save the value that was set by an environment variable. This could be implemented through the save_config function. But if a user calls the .write() method directly on the ConfigObj object it won't work correctly. I could be fine with that as long as we make sure the user always uses our API...

    Another possibility is that there's a recent patch for ConfigObj that makes it possible to subclass the Section class throughout ConfigObj. This would allow us to use a simple Section subclass to implement this functionality consistently.

  3. eteq commented on Dec 8, 2011

    @eteq
    Member

    I don't fully understand the problem... why would the user be calling ConfObj.write()? As the ConfigurationItem class is written, they should always either call ConfigurationItem.save() on a particular item or astropy.config.configs.save_config(). The idea I have here is to make it so the user doesn't even know ConfigObj is actually even present - all they know about is the astropy.config.configs stuff. So you could add a method to the ConfigurationItem that checks if the current item setting came from the environment variable or not, and if it does, temporarily put back in whatever it's "supposed" to be when saving. Not terribly elegant, but that was part of the reason I thought we wanted an interface where we control all the entry points...

  4. embray commented on Dec 8, 2011

    @embray
    MemberAuthor

    @eteq I already have ConfigurationItem doing as you suggested, and that works. So you're right, as long as they only use the API we give them I guess there's no problem.

    The only place that there is a problem is that get_config() still returns a ConfigObj object. That's where I was running into issues... I don't really mind that it does, though I suppose if we really wanted we could convert it to a normal dict--that would control things even more.

  5. eteq commented on Dec 8, 2011

    @eteq
    Member

    Ahh, now I see what you're saying. I think it's good to have access to the actual ConfgObj just in case it's needed. But given what you've said here, we should probably add the following to the get_config docstring:

    .. warning::
        Calling :meth:`ConfigObj.write` directly may incorrectly save your configuration files.  Instead, always use `ConfigurationItem.save` or `save_config` to save configuration settings.
    

    (note that I'm doing the :meth: thing in ConfigObj becaue configobj doesn't seem to do intersphinx, so sphinx won't know it's a function and do the () thing without explicit marking)

  6. embray commented on Dec 8, 2011

    @embray
    MemberAuthor

    Sounds fine. Also, I usually use :meth:, :func:, and friends even when they can be otherwise be inferred, just in case...

  7. eteq commented on Dec 8, 2011

    @eteq
    Member

    Ok, cool. Once you've gotten this updated, maybe convert this issue to a pull request (if you don't know how to do this, see #98 (comment) or the comment above that one by @mdboom) so we can review and merge that way?

  8. embray commented on Dec 8, 2011

    @embray
    MemberAuthor

    Cool, so there is a way! I don't know why they don't just add this to the UI already--it seems like a commonly requested feature.

  9. eteq commented on Dec 9, 2011

    @eteq
    Member

    I've thought the same thing - I mean, it's not really that hard to do it once you dig into the API, but it would be so simple to just add a button... I imagine they'll add it sooner or later.

  10. added a commit that references this issue on Feb 15, 2012
  11. astrofrog commented on May 3, 2014

    @astrofrog
    Member

    Re-scheduling to 1.0 since we should wait until the new APE3 changes are settled.

  12. modified the milestones: v1.0.0, v0.4.0 on May 3, 2014
  13. astrofrog commented on Jan 14, 2015

    @astrofrog
    Member

    Removing the 1.0 milestone since this is low priority and there has been no work on it recently.

  14. removed this from the v1.0.0 milestone on Jan 14, 2015
  15. added a commit that references this issue on Jan 23, 2015
  16. eteq commented on May 8, 2015

    @eteq
    Member

    @mdboom - I was re-visiting some random old issues... do you think this still makes sense in the context of the new config system? Or should we just close this?

  17. pllim commented on May 11, 2017

    @pllim
    Member

    I say we just close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions