SatelliteQE / satellite-populate

This new module adds tools and commands to populate and validate entities in the system based in YAML file.
Other
8 stars 10 forks source link

RFE: populate_with decorator #6

Closed ldjebran closed 7 years ago

ldjebran commented 7 years ago

the context arg of the decorator is too ambiguous, it can have the sense whether to use context as True bool value or used as context name string value to be injected as arg in the called function.

def populate_with(data, context=None, **extra_options):
    ...

propose to split the args by meaning by refactoring like: two naming options: inject_context or use_context

def populate_with(data, inject_context=False, context_name='context', **extra_options):
    ...

or

def populate_with(data, use_context=False, context_name='context', **extra_options):
    ...

also propose to add a customized context wrapper in the decorator wrapper : rename the _wrapcontext function to _default_contextwrapper and the decorator function became like this

def populate_with(data, use_context=False, context_name='context', context_wrapper=default_context_wrapper, **extra_options):
    ...
     result = populate(data, **extra_options)
     if use_context:
         if context_wrapper:
             context = context_wrapper(result)
        else:
            context = result
        kwargs[context_name] = context
    ...
ldjebran commented 7 years ago

The context wrapper customization will also give us the possibility to return the pure result when needed by setting context_wrapper=None

ldjebran commented 7 years ago

@rochacbruno , @omaciel should I go with this refactoring. If yes, what are the preferences: inject_context or use_context

rochacbruno commented 7 years ago

@ldjebran I think we can have only one argument context_name="foo" and if this is not None the context is passed to the funtion as foo=context.

Go ahead if you want, and fetch before, because I just updated the nailgun settings part.

ldjebran commented 7 years ago

close issue as done