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

Added default value for 'detail' param into 'ValidationError' exception #5342

Merged
merged 1 commit into from Sep 26, 2017
Merged

Added default value for 'detail' param into 'ValidationError' exception #5342

merged 1 commit into from Sep 26, 2017

Conversation

ghost
Copy link

@ghost ghost commented Aug 19, 2017

Description

In the current implementation, detail is a positional argument. But the code in line 127 expects an keyword. If I do not pass the argument detail when creating an instance of ValidationError, then we get:

TypeError: __init__() missing 1 required positional argument: 'detail'

but not _('Invalid input.')

@carltongibson
Copy link
Collaborator

It could be that the intended API here is to explicitly pass None if you don't want to pass detail. (In X years I've never not passed a string here...) @tomchristie: what's your thought?

@carltongibson carltongibson added this to the 3.7.0 Release milestone Sep 26, 2017
Copy link
Collaborator

@carltongibson carltongibson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I'm going to say we'll have this. I can't see any harm in it and ValidationError() is more natural than ValidationError(None) — although, as I say, I've don't think I've ever not passed a detail there.

@rokkerruslan Thanks for the input!

@carltongibson carltongibson merged commit ab7e5c4 into encode:master Sep 26, 2017
carltongibson added a commit that referenced this pull request Sep 26, 2017
@ghost ghost deleted the fix_default_value_for_validation_error_exception branch September 27, 2017 06:33
carltongibson added a commit that referenced this pull request Sep 27, 2017
carltongibson added a commit that referenced this pull request Sep 28, 2017
carltongibson added a commit that referenced this pull request Oct 5, 2017
carltongibson added a commit that referenced this pull request Oct 5, 2017
carltongibson pushed a commit that referenced this pull request Oct 6, 2017
* Set version number for 3.7.0 release

* Rename release notes section

Moved issue links to top for easier access.
(Can move back later)

* Add release note for #5273

* Add release note for #5440

* Add release note for #5265

Strict JSON handling

* Add release note for #5250

* Add release notes for #5170

* Add release notes for #5443

* Add release notes for #5448

* Add release notes for #5452

* Add release not for #5342

* Add release notes for 5454

* Add release notes for #5058 & #5457

Remove Django 1.8 & 1.9 from README and setup.py

* Release notes for merged 3.6.5 milestone tickets

Tickets migrated to 3.7.0 milestone.

* Add release notes for #5469

* Add release notes from AM 2ndOct

* Add final changes to the release notes.

* Add date and milestone link

Move issue links back to bottom.

* Update translations from transifex

* Begin releae anouncement

* Add release note for #5482

* 3.7 release announcement & related docs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant