Support kwargs in marshmallow validator #516

mamalos · 2019-09-12T10:38:02Z

With this patch, keyword arguments are supported when instantiating the marshmallow schema (#511).

This is an example of how to use it:

def my_validator(request, **kwargs):
    if request.method == 'POST':
        sch = MyPOSTSchema
        kwargs['schema_kwargs'] = {'many': True}
    elif request.method == 'PATCH':
        sch = MyPATCHSchema
    kwargs['schema'] = sch
    return marshmallow_body_validator(request, **kwargs)

class MyView(object):
    @view(validators=(my_validator,))
    def patch(self):
        return self._update()

So, if someone passes schema_kwargs dictionary in a marshmallow validator, it is expanded in the schema's constructor when it is instantiated.

Pinging @ergo for a second opinion.

Natim · 2019-09-12T13:30:35Z

Is this a duplicate of #515? If you want to keep improving your PR you can add commit to your branch.

mamalos · 2019-09-13T08:45:32Z

Hi @Natim, no it's not a duplicate; #515 fixes a compatibility issue with marshmallow v.3.x (which prohibited its use through cornice's marshmallow helper), this one tries to add some functionality to marshmallow validator, so that one can exploit many more of its features (as I just saw it was also requested as a feature in #501 around 7 months ago).

I'll have to see what broke things in the tests and see if I can fix it.

Natim · 2019-09-13T08:46:19Z

Ok thank you for the clarification, I have to say I don't know much about marshmallow support in Cornice.

mamalos · 2019-09-13T08:54:17Z

Np, me neither :).

In terms of failing tests, as I saw, I need to add some tests to keep coverage at 100%. Will do it later when I find some free time.

mamalos · 2019-09-13T09:46:37Z

OK, I've added some comments in validators/_marshmallow.py of how it now supports schema_kwargs variable from views.

I've replaced one of the existing tests that used an extra schema that added many=True in its constructor, with one that uses the new functionality (ie passing {'many': True} as a schema_kwargs variable when calling marshmallow_body_validator) so that the tests meet the 100% coverage requirement.

mamalos · 2019-09-13T10:01:44Z

If this PR gets accepted, we should probably update cornice documentation to include this functionality. Not sure if the docs (https://cornice.readthedocs.io) are maintained this repo, if so I have no problem of updating the relevant section.

Natim · 2019-09-13T12:08:09Z

Yes the docs are there: https://github.com/Cornices/cornice/tree/master/docs

tests/validationapp.py

leplatrem · 2019-10-01T13:28:52Z

What's the status of this?

Also, could you please add a CHANGELOG entry and yourself to the CONTRIBUTORS?

mamalos · 2019-10-01T18:52:40Z

Hi, I need to check if I've included the changes suggested by @Natim, push them in case I haven't done so, and I think we're done with it.

And as you suggested, I should update the CHANGELOG and the CONTRIBUTORS files accordingly.

…yword arguments on marshmallow schema instantiation

mamalos · 2019-11-18T12:25:32Z

OK, I've updated CONTRIBUTORS.txt and CHANGES.txt and also tried to see if @Natim suggestion would work but unfortunately it didn't.

@Natim, when trying to pass your version of the code, many tests are failing. Here's the error's I'm getting in one of such failing test:

Traceback (most recent call last):
  File "/usr/lib/python2.7/unittest/case.py", line 329, in run
    testMethod()
  File "/home/mamalos/my_programs/contrib/github/cornice/tests/test_validation.py", line 497, in test_bound_schema_multiple_calls
    app = self.make_ordinary_app()
  File "/home/mamalos/my_programs/contrib/github/cornice/tests/test_validation.py", line 477, in make_ordinary_app
    return TestApp(main({}))
  File "/home/mamalos/my_programs/contrib/github/cornice/tests/validationapp.py", line 524, in main
    return CatchErrors(config.make_wsgi_app())
  File "/home/mamalos/my_programs/contrib/github/cornice/.tox/py27/local/lib/python2.7/site-packages/pyramid/config/__init__.py", line 891, in make_wsgi_app
    self.commit()
  File "/home/mamalos/my_programs/contrib/github/cornice/.tox/py27/local/lib/python2.7/site-packages/pyramid/config/actions.py", line 152, in commit
    self.action_state.execute_actions(introspector=self.introspector)
  File "/home/mamalos/my_programs/contrib/github/cornice/.tox/py27/local/lib/python2.7/site-packages/pyramid/config/actions.py", line 293, in execute_actions
    action = next(action_iter, None)
  File "/home/mamalos/my_programs/contrib/github/cornice/.tox/py27/local/lib/python2.7/site-packages/pyramid/config/actions.py", line 425, in resolveConflicts
    discriminator = undefer(action['discriminator'])
  File "/home/mamalos/my_programs/contrib/github/cornice/.tox/py27/local/lib/python2.7/site-packages/pyramid/registry.py", line 301, in undefer
    v = v.resolve()
  File "/home/mamalos/my_programs/contrib/github/cornice/.tox/py27/local/lib/python2.7/site-packages/pyramid/registry.py", line 292, in resolve
    return self.value
  File "/home/mamalos/my_programs/contrib/github/cornice/.tox/py27/local/lib/python2.7/site-packages/pyramid/decorator.py", line 43, in __get__
    val = self.wrapped(inst)
  File "/home/mamalos/my_programs/contrib/github/cornice/.tox/py27/local/lib/python2.7/site-packages/pyramid/registry.py", line 287, in value
    result = self.func()
  File "/home/mamalos/my_programs/contrib/github/cornice/.tox/py27/local/lib/python2.7/site-packages/pyramid/config/views.py", line 940, in discrim_func
    self._check_view_options(**dvals)
  File "/home/mamalos/my_programs/contrib/github/cornice/.tox/py27/local/lib/python2.7/site-packages/pyramid/config/views.py", line 1209, in _check_view_options
    raise ConfigurationError('Unknown view options: %s' % (kw,))
ConfigurationError: Unknown view options: {'schema_kwargs': {'many': True}}

Here's the code I've tried:

    @m_group_signup.post(
        schema=MSignupSchema, schema_kwargs={'many': True},
        validators=(marshmallow_body_validator,))
    def m_group_signup_post(request):
        return {'data': request.validated}

Not sure why it happens -your suggestion seems very reasonable to me too-, but unless you want to dig it any futher, I suggest we keep the current version that's passing the tests - and if you're cool with that, obviously.

mamalos · 2019-12-03T06:09:04Z

Hi there (@Natim, @leplatrem),

Maybe it's time for somebody to review and accept the change?

leplatrem

It looks good to me.

Although I think we should clarify our API regarding schema instance versus class + kwargs. See for example #523

cornice/validators/_marshmallow.py

Co-Authored-By: Mathieu Leplatre <[email protected]>

leplatrem · 2019-12-04T10:07:26Z

cornice/validators/_marshmallow.py

+        # see if the user wants to set any keyword arguments for their schema
+        schema_kwargs = {}
+        if 'schema_kwargs' in kwargs:
+            schema_kwargs = kwargs.get('schema_kwargs', {})


Sorry this code suggestion thing didn't work as I expected. You can remove the above lines

Sorry, you lost me in this. The truth is that I accepted this change without seeing it too deeply, I thought it was in a different spot. Now I'm a bit confused, has it been merged with my code or not?

Just that these 3 lines can be replaced by the last one

schema_kwargs = kwargs.get('schema_kwargs', {})

Is it OK now? I'm not familiar with these github's features (merging suggested changes and editing files within github), that's why I'm asking.

So, I've edited the file within github to only contain schema_kwargs = kwargs.get('schema_kwargs', {}) and I suppose it's OK now, but I'm not sure I saw the checks running again.

EDIT:
I've clicked on the checks and the logs show they have run successfully after I made the last changes within github, so I suppose we're OK.

mamalos · 2019-12-04T10:11:49Z

Hi @leplatrem and thanks for the comments and suggestions.

In terms of class vs instance, I haven't changed anything in the current implementation. Marshmallow is different from colander in this matter. It allows you to reuse the same validator class and tailor it appropriately upon instantiation (using constructor's args/kwargs), which is one of its very strengths.

So, despite the fact that the provided PR is not very "elegant" (having a schema_kwargs as a keyword argument is not the "pretiest" thing), it allows for proper integration of marshmallow with cornice and helps very much in code reuse.

Once this PR (and #515) are complete and accepted, I'll also update the documentation in order for people to know they can use it.

leplatrem

👏 👏

Natim reviewed Sep 13, 2019

View reviewed changes

tests/validationapp.py Show resolved Hide resolved

support for schema_kwargs keyword argument dictionary which passes ke…

3d46848

…yword arguments on marshmallow schema instantiation

mamalos force-pushed the support-kwargs-in-marshmallow-validator branch from 537cba5 to 3d46848 Compare November 18, 2019 11:36

leplatrem reviewed Dec 3, 2019

View reviewed changes

cornice/validators/_marshmallow.py Outdated Show resolved Hide resolved

cornice/validators/_marshmallow.py Outdated Show resolved Hide resolved

mamalos and others added 2 commits December 4, 2019 12:04

Update cornice/validators/_marshmallow.py

eafd6ff

Co-Authored-By: Mathieu Leplatre <[email protected]>

Update cornice/validators/_marshmallow.py

975ba4a

Co-Authored-By: Mathieu Leplatre <[email protected]>

leplatrem reviewed Dec 4, 2019

View reviewed changes

Update _marshmallow.py

236a660

leplatrem approved these changes Dec 5, 2019

View reviewed changes

leplatrem merged commit 9df75f5 into Cornices:master Dec 5, 2019

mamalos deleted the support-kwargs-in-marshmallow-validator branch January 7, 2020 09:36

mamalos mentioned this pull request Jan 7, 2020

[Feature request] It would be nice if one could pass marshmallow's keyword arguments when creating a schema object. #511

Closed

mamalos mentioned this pull request Feb 26, 2020

marshmallow_validator seems to break with marshmallow version 3.0.1 #514

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support kwargs in marshmallow validator #516

Support kwargs in marshmallow validator #516

mamalos commented Sep 12, 2019

Natim commented Sep 12, 2019

mamalos commented Sep 13, 2019

Natim commented Sep 13, 2019

mamalos commented Sep 13, 2019

mamalos commented Sep 13, 2019

mamalos commented Sep 13, 2019 •

edited

Loading

Natim commented Sep 13, 2019

leplatrem commented Oct 1, 2019

mamalos commented Oct 1, 2019

mamalos commented Nov 18, 2019 •

edited

Loading

mamalos commented Dec 3, 2019

leplatrem left a comment

leplatrem Dec 4, 2019

mamalos Dec 4, 2019

leplatrem Dec 4, 2019

mamalos Dec 5, 2019 •

edited

Loading

mamalos commented Dec 4, 2019 •

edited

Loading

leplatrem left a comment

Support kwargs in marshmallow validator #516

Support kwargs in marshmallow validator #516

Conversation

mamalos commented Sep 12, 2019

Natim commented Sep 12, 2019

mamalos commented Sep 13, 2019

Natim commented Sep 13, 2019

mamalos commented Sep 13, 2019

mamalos commented Sep 13, 2019

mamalos commented Sep 13, 2019 • edited Loading

Natim commented Sep 13, 2019

leplatrem commented Oct 1, 2019

mamalos commented Oct 1, 2019

mamalos commented Nov 18, 2019 • edited Loading

mamalos commented Dec 3, 2019

leplatrem left a comment

Choose a reason for hiding this comment

leplatrem Dec 4, 2019

Choose a reason for hiding this comment

mamalos Dec 4, 2019

Choose a reason for hiding this comment

leplatrem Dec 4, 2019

Choose a reason for hiding this comment

mamalos Dec 5, 2019 • edited Loading

Choose a reason for hiding this comment

mamalos commented Dec 4, 2019 • edited Loading

leplatrem left a comment

Choose a reason for hiding this comment

mamalos commented Sep 13, 2019 •

edited

Loading

mamalos commented Nov 18, 2019 •

edited

Loading

mamalos Dec 5, 2019 •

edited

Loading

mamalos commented Dec 4, 2019 •

edited

Loading