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

Meta Attribute for Requiring Settings #6

Merged
merged 5 commits into from Jan 13, 2013

Conversation

matthewwithanm
Copy link
Contributor

This adds support for a new (optional) AppConf.Meta attribute named "required." The attribute is a list of (unprefixed) settings that must be defined. Not defining any of the settings will result in an ImproperlyConfigured exception being raised.

@@ -63,6 +64,11 @@ Reference
For example, ``acme`` would turn into settings like
``ACME_SETTING_1``.

.. attribute:: required

Copy link
Member

Choose a reason for hiding this comment

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

Can you add a .. versionadded:: 0.6 directive to the block?

@jezdez
Copy link
Member

jezdez commented Jun 5, 2012

Actually, now that I think about it, what would you think about a different API:

from appconf import AppConf, required

class MyConf(AppConf):

    SOMETHING_AWESOME = required('truly-epic')

    OTHER_THING = required(['foo', 'bar'])

    A_INTERESTING_FLAG = required(True)

What required would basically do is to mark the default value with a boolean and that check could be done during initialization.

@matthewwithanm
Copy link
Contributor Author

Honestly, I'm not a fan; mainly because I can't think of a use case where you'd want to require a value and give it a default (since a default value means the user doesn't have to set one).

@jezdez
Copy link
Member

jezdez commented Aug 29, 2012

Good point, forget what I said, we'll do this like you proposed. Do you think it would be sensible to also allow the arguments passed to the required option to be lower cased (to be consistent with the configure_* methods)?

@matthewwithanm
Copy link
Contributor Author

Yeah, that makes sense to me. I don't think we need to change anything though since prefixed_name is already doing the job by uppering the setting name.

@travisbot
Copy link

This pull request fails (merged f0534ae into bf14a76).

@matthewwithanm
Copy link
Contributor Author

Anything holding this up that I could take care of?

@jezdez
Copy link
Member

jezdez commented Jan 13, 2013

Nope, sorry!

jezdez added a commit that referenced this pull request Jan 13, 2013
Meta Attribute for Requiring Settings
@jezdez jezdez merged commit 0fbf0ec into django-compressor:develop Jan 13, 2013
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.

None yet

3 participants