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

Support custom Schema class for argmap2schema #375

Closed
ThiefMaster opened this issue Mar 15, 2019 · 4 comments

Comments

@ThiefMaster
Copy link
Contributor

commented Mar 15, 2019

This could be done by passing the Schema class to the Parser constructor.

That way someone could use a custom Schema class that has a pre_load hook which would then run when webargs processes data as well. My usecase for this is that my application has a plugin system and it'd be nice to let plugins not only modify the data that gets serialized (by marshmallow itself) but also the data it loads (e.g. to inject values that are usually required to be sent by the client, but overridden by the plugin).

I wouldn't mind sending a PR for this.

@lafrech

This comment has been minimized.

Copy link
Member

commented Mar 15, 2019

This makes sense. I have no objection.

It could be like

class Parser(object):

    DEFAULT_SCHEMA_CLS = ma.Schema
    
    def __init__(self, locations=None, error_handler=None, schema_cls=None):
        ...
        self.schema_cls = schemas_cls or DEFAULT_SCHEMA_CLS

This way it could be parametrized either by modifying the class attribute or by passing an init parameter.

Note that argmap2schema is now dict2schema.

If would be nice if we could settle on the rework in #374 before adding more features but I don't think the proposal here would be fundamentally broken by the rework, so it's not a blocker.

@ThiefMaster

This comment has been minimized.

Copy link
Contributor Author

commented Mar 15, 2019

Since this is fully backwards-compatible I was hoping for this to make it into v5.2 if such a version happens to be released before 6.0 ;)

Is there a benefit to allow overriding this both on the class and instance level? If someone subclasses, they could already override the constructor and pass a custom default value there.

@lafrech

This comment has been minimized.

Copy link
Member

commented Mar 15, 2019

Since this is fully backwards-compatible I was hoping for this to make it into v5.2 if such a version happens to be released before 6.0 ;)

Sure, no problem. Sorry, I didn't mean to put #374 in the way. The feature you're talking about is a simple addition that could be shipped in a 5.2.

Is there a benefit to allow overriding this both on the class and instance level? If someone subclasses, they could already override the constructor and pass a custom default value there.

I was mimicking DEFAULT_LOCATIONS and I find it a little nicer when subclassing but I have no strong feeling about it.

ThiefMaster added a commit to ThiefMaster/webargs that referenced this issue Mar 15, 2019

@ThiefMaster

This comment has been minimized.

Copy link
Contributor Author

commented Mar 15, 2019

#376

@sloria sloria closed this in 458a3b7 Mar 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.