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

bug: webargs 5.0 introduces incompatibility with Flask < 1.0 #355

Closed
hoatle opened this issue Jan 11, 2019 · 3 comments

Comments

@hoatle
Copy link

commented Jan 11, 2019

https://github.com/marshmallow-code/webargs/blob/5.0.0/webargs/flaskparser.py#L63

_get_data_for_json is only available since Flask >= 1.0

for Flask < 1.0, there is an error as follows:

  File "/usr/local/lib/python2.7/site-packages/webargs/flaskparser.py", line 63, in parse_json
    data = req._get_data_for_json(cache=True)
  File "/usr/local/lib/python2.7/site-packages/werkzeug/local.py", line 347, in __getattr__
    return getattr(self._get_current_object(), name)
AttributeError: 'Request' object has no attribute '_get_data_for_json'

I had to downgrade webargs to 4.4.1 to get it work.

So you need to update this framework requirement https://github.com/marshmallow-code/webargs/blob/dev/setup.py#L11 or update the code for the backward compatibility.

IMHO, using _get_data_for_json should be avoided because it's considered private and can be changed/removed anytime.

@lafrech lafrech changed the title bug: webargs 5.0 introduces incompatibility with Flask <= 1.0 bug: webargs 5.0 introduces incompatibility with Flask < 1.0 Jan 11, 2019

@lafrech

This comment has been minimized.

Copy link
Member

commented Jan 11, 2019

Thanks for reporting.

I don't fully understand the reason @sloria did it this way.

        # We decode the json manually here instead of
        # using req.get_json() so that we can handle
        # JSONDecodeErrors consistently

Apparently, catching BadRequest (which is both flask<1.0 and flask 1.0) was not satisfying.

I wouldn't mind dropping flask < 1.0 if we had to since flask 1 has been out for a while and from my experience, there wasn't much trouble migrating.

Regarding the use of a protected method, I suppose we could call

self.get_data(cache=cache)

Perhaps we could even add a bit of code from https://github.com/pallets/flask/blob/0.12-maintenance/flask/wrappers.py#L21 to preserve flask 0.x compatibility.

@sloria

This comment has been minimized.

Copy link
Member

commented Jan 11, 2019

I don't fully understand the reason @sloria did it this way.

            # We decode the json manually here instead of
            # using req.get_json() so that we can handle
            # JSONDecodeErrors consistently
            data = req._get_data_for_json(cache=True)

The reason it is done this way is so that a JSONDecodeError would be raised and we could therefore check e.doc and pass the exception to handle_invalid_json_error (rather than passing a BadRequest). This makes the API consistent with the behavior of the other parsers.

            try:
                self._cache["json"] = json_data = core.parse_json(data)
            except json.JSONDecodeError as e:
                if e.doc == "":
                    return core.missing
                else:
                    return self.handle_invalid_json_error(e, req)

Regarding the use of a protected method, I suppose we could call self.get_data(cache=cache)

Yeah, this seems like the way to go. Didn't see that method before.

I'm up for dropping Flask 1.0, but that's a breaking change for a major release. For now, let's use get_data, which has existed since werkzeug 0.9, which is 5.5 years old.

@sloria sloria self-assigned this Jan 11, 2019

sloria added a commit that referenced this issue Jan 11, 2019

Fix Flask<1.0 compatibility
Use Request.get_data rather than the private _get_data_for_json,
which doesn't exist on pre-1.0 Flask versions.

Confirmed tests are passing for both Flask 0.12.4 and 1.0.2

close #355

@sloria sloria closed this in #356 Jan 11, 2019

@sloria

This comment has been minimized.

Copy link
Member

commented Jan 11, 2019

Fix is released in 5.1.0.

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