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

Add custom error message when django_get_or_create is missing an input. #292

Merged
merged 1 commit into from Apr 18, 2016

Conversation

Projects
None yet
3 participants
@rrauenza

Ok, I couldn't figure out how to squash it, so just recreated the change after rewinding and resyncing with upstream.

@jeffwidman

This comment has been minimized.

Show comment
Hide comment
@jeffwidman

jeffwidman Apr 12, 2016

Member

Continuation of #290

LGTM, though I'm going ot hold off merging until @rbarrois looks at it as he's the Django point person.

Member

jeffwidman commented Apr 12, 2016

Continuation of #290

LGTM, though I'm going ot hold off merging until @rbarrois looks at it as he's the Django point person.

@jeffwidman jeffwidman added the Django label Apr 13, 2016

@rrauenza

This comment has been minimized.

Show comment
Hide comment
@rrauenza

rrauenza Apr 14, 2016

Sure. The underlying problem is trying to populate the Django get_or_create args, but when an initial value isn't provided, it just throws a KeyError -- so I just throw a more informative error message instead pointing to the root cause.

Sure. The underlying problem is trying to populate the Django get_or_create args, but when an initial value isn't provided, it just throws a KeyError -- so I just throw a more informative error message instead pointing to the root cause.

@rrauenza

This comment has been minimized.

Show comment
Hide comment
@rrauenza

rrauenza Apr 14, 2016

@rbarrois , alternatively key_fields[field] = kwargs.pop(field) could be changed to key_fields[field] = kwargs.pop(field, None)

That's definitely more of a design decision there... because None could be a valid value in the Model.

@rbarrois , alternatively key_fields[field] = kwargs.pop(field) could be changed to key_fields[field] = kwargs.pop(field, None)

That's definitely more of a design decision there... because None could be a valid value in the Model.

@rbarrois

This comment has been minimized.

Show comment
Hide comment
@rbarrois

rbarrois Apr 18, 2016

Member

@rrauenza This looks good :)

I'm merging it, but it would be great if you could add a small test checking that this works as expected: the amount of meta-programming inherent to factory_boy is known to cause unexpected behaviour :/

Member

rbarrois commented Apr 18, 2016

@rrauenza This looks good :)

I'm merging it, but it would be great if you could add a small test checking that this works as expected: the amount of meta-programming inherent to factory_boy is known to cause unexpected behaviour :/

@rbarrois rbarrois merged commit 187d849 into FactoryBoy:master Apr 18, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@rrauenza

This comment has been minimized.

Show comment
Hide comment

See #297

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment