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

CementApp Keyword Args Fail if No Meta Default #395

Closed
derks opened this Issue Jul 13, 2016 · 2 comments

Comments

Projects
None yet
1 participant
@derks
Member

derks commented Jul 13, 2016

Extensions can simplify setup/configuration by honoring CementApp.Meta options, rather than making the develop sub-class/re-register handler to override the handlers meta options. However, if the meta option isn't already defined in CementApp.Meta then it will not take them via **kwargs to CementApp.

For example:

app = CementApp(extensions=['handlebars'], handlebars_helpers={'foo' : bar})

The above should result in CementApp.Meta.handlebars_helpers == {'foo' : bar} but it doesn't because the handlebars_helpers meta data doesn't exist so it is ignored. Extensions should be able to extend CementApp.Meta same as they can the app with CementApp.extend().

@derks derks added this to the 2.10.0 Stable milestone Jul 13, 2016

@derks derks self-assigned this Jul 13, 2016

@derks

This comment has been minimized.

Show comment
Hide comment
@derks

derks Jul 13, 2016

Member

A better solution than supporting CementApp.Meta.extension_meta_option would be to have a means of passing **kwargs to the handlers that Cement is setting up, being that it is calling them as SomeHandler() without anything anyway.

Maybe:

LOGGING_META = {
    'console_format' : '%{message}',
}

class MyApp(CementApp):
    Class Meta:
        label = 'myapp'
        handler_meta = {
            'log.logging' : LOGGING_META,
        }

Where log.loggin is the <interface>.<handler> label (same default we use for config file sections).

The alternative is currently:

from cement.ext.ext_logging import LoggingLogHandler

class MyLogHandler(LoggingLogHandler):
    class Meta:
        label = 'my_log_handler'
        console_format = '%{message}'

class MyApp(CementApp):
    Class Meta:
        label = 'myapp'
        log_handler = 'my_log_handler'
        handlers = [
            MyLogHandler,
        ]

Which is excessive just to override a meta option and nothing else. This is different than how configuration settings are handle where we first setup config defaults. Handler meta options are sometimes the same (overriden by) config settings.. but many times are not, and are only intended to be modified by the developer to modify how a class works. It's not a user setting to modify.

Member

derks commented Jul 13, 2016

A better solution than supporting CementApp.Meta.extension_meta_option would be to have a means of passing **kwargs to the handlers that Cement is setting up, being that it is calling them as SomeHandler() without anything anyway.

Maybe:

LOGGING_META = {
    'console_format' : '%{message}',
}

class MyApp(CementApp):
    Class Meta:
        label = 'myapp'
        handler_meta = {
            'log.logging' : LOGGING_META,
        }

Where log.loggin is the <interface>.<handler> label (same default we use for config file sections).

The alternative is currently:

from cement.ext.ext_logging import LoggingLogHandler

class MyLogHandler(LoggingLogHandler):
    class Meta:
        label = 'my_log_handler'
        console_format = '%{message}'

class MyApp(CementApp):
    Class Meta:
        label = 'myapp'
        log_handler = 'my_log_handler'
        handlers = [
            MyLogHandler,
        ]

Which is excessive just to override a meta option and nothing else. This is different than how configuration settings are handle where we first setup config defaults. Handler meta options are sometimes the same (overriden by) config settings.. but many times are not, and are only intended to be modified by the developer to modify how a class works. It's not a user setting to modify.

derks added a commit that referenced this issue Jul 14, 2016

@derks

This comment has been minimized.

Show comment
Hide comment
@derks

derks Jul 14, 2016

Member

This is now implemented in master. Example here is overriding the JsonOutputHandler meta option for json_module so that we can use ujson instead of the standard json:

from cement.core.foundation import CementApp
from cement.utils.misc import init_defaults

META = init_defaults('output.json')
META['output.json']['json_module'] = 'ujson'


class MyApp(CementApp):
    class Meta:
        label = 'myapp'
        extensions = ['json']
        output_handler = 'json'
        meta_defaults = META
Member

derks commented Jul 14, 2016

This is now implemented in master. Example here is overriding the JsonOutputHandler meta option for json_module so that we can use ujson instead of the standard json:

from cement.core.foundation import CementApp
from cement.utils.misc import init_defaults

META = init_defaults('output.json')
META['output.json']['json_module'] = 'ujson'


class MyApp(CementApp):
    class Meta:
        label = 'myapp'
        extensions = ['json']
        output_handler = 'json'
        meta_defaults = META

@derks derks closed this Jul 14, 2016

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