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

default initialization of configurable traits from os.environ #158

Closed
wants to merge 1 commit into from

Conversation

rmorshea
Copy link
Contributor

Configurables hooks into a call from the metaclass to setup_class in order to setup default values from os.environ. This in turn calls TraitType.set_default_from_envvar to apply that default value to the TraitType if the key 'envvar' is in its metadata.

@@ -0,0 +1,88 @@
"""A dict-like table mapping keys to values based on indexing"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this file was added by mistake.

@rmorshea rmorshea force-pushed the envvar_defaults branch 4 times, most recently from 2caab34 to a981a0c Compare January 3, 2016 07:05
@rmorshea
Copy link
Contributor Author

rmorshea commented Jan 4, 2016

I've included TraitType.set_default_from_envvar to apply default values from environment variables, but I have two questions about how I've implemented that hook:

  • Should I check a trait's metadata for 'envvar' (as I am now) or an attribute envvar to determine whether it needs a default value to be set from an environment variable?

Having envvar be an attribute, and an explicit entry in TraitType.__init__ makes it clear that environment variable defaults are a feature, however it might introduce confusion, because it is only a feature of traits attached to configurables.

  • Should I apply environment variable defaults through setup_class (as I am now) or setup_instance?

Using setup_class makes sense to me, and would be slightly more efficient, but I'm not really familiar with conventions surrounding how applications are typically configured (i.e. whether configurations should exist at the class level or their instances).

@minrk minrk modified the milestone: 4.2 Jan 13, 2016
@minrk minrk modified the milestones: 4.2, 4.3 Mar 14, 2016
@Carreau
Copy link
Member

Carreau commented Jun 21, 2016

This seem to be stale a bit, and start to look like over engineering what traitlets can (and should do).
This seem also like it's pretty easy to get by using @default function.

So I'm curious if

  • we really want that ?
  • is it worked on ?

Because that seem like quite a bit of maintenance for a potentialy really low use case, and we are spread thin, so I'm leaning toward "no new feature" at that point.

@rmorshea
Copy link
Contributor Author

@Carreau, should probably bring up whether this is needed in #157. As far as @default, it really doesn't add anything different than _<name>_default. However what it does allow for is an @envvar_default(name, envvar) decorator in configurable.py. The decorator could set trait metadata, and pass the envvar string into the default generator.

@rmorshea
Copy link
Contributor Author

I'll close this one and make a new pr for @envvar_default.

@rmorshea rmorshea closed this Jun 22, 2016
@rmorshea rmorshea deleted the envvar_defaults branch June 22, 2016 19:57
@ankostis ankostis mentioned this pull request Aug 12, 2017
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 this pull request may close these issues.

3 participants