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

abort: Why is data conditionally added? #723

Open
sloria opened this issue Nov 28, 2017 · 2 comments
Open

abort: Why is data conditionally added? #723

sloria opened this issue Nov 28, 2017 · 2 comments

Comments

@sloria
Copy link

sloria commented Nov 28, 2017

if len(kwargs):
e.data = kwargs

Why check len(kwargs) here? This requires handling code to check for the existence of the data attribute:

    if hasattr(error, data):
        error.data.get(my_kwarg, default_val)

Can the check be removed?

For context: webargs has a similar abort function, and this issue came up in a PR: marshmallow-code/webargs#184 . I think it's a good idea to remove the check. Before I merge that, though, I thought I'd ask about the original rationale.

@joshfriend
Copy link
Member

Not sure TBH, but the error handler adds a default data value if one doesn't exist:

data = getattr(e, 'data', default_data)

If you remove it do the tests still pass?

@sloria
Copy link
Author

sloria commented Nov 29, 2017

@joshfriend

No, this test fails.

def test_abort_no_data(self):
try:
flask_restful.abort(404)
assert False # We should never get here
except Exception as e:
self.assertEquals(False, hasattr(e, "data"))

Though if you need to check if data were passed, you could just do

data = e.data or default_data

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants