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

Fix error when adding custom fields generators to bakery via settings file #58

Merged
merged 10 commits into from
Jan 22, 2020

Conversation

berinhard
Copy link
Member

Fixes #18

The many comments on #18 describes why this work was required, but the short summary is that the instruction in the docs to set generators for custom fields via settings.py wasn't working. Django raises the exception AppRegistryNotReady if the code tries to import anything related to the DB before properly creating the models.

Model bakery wasn't respecting this rule because it was importing the ContentType model and a few django.contrib.gis models too. This PR fixes this by replacing direct imports by module's imports and by moving others imports to be done during the execution time.

@berinhard
Copy link
Member Author

@amureki and @anapaulagomes this is a very old bug that I finally sat down to fix and, thanks to jah, it was waaaaay more trivial than the issue suggested ¬¬. Whenever you have time to review it'd be nice! I think we can release a new version after this PR is merged.

Copy link
Contributor

@anapaulagomes anapaulagomes left a comment

Choose a reason for hiding this comment

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

Wow, @berinhard is on fire! 🔥

I only miss a test for it. I wonder if would be possible to test it adding custom fields to a settings file since we don't have one in the current setup (I'm on vacation, so I've skipped testing any approach locally, sorry :P)

model_bakery/baker.py Outdated Show resolved Hide resolved
@berinhard
Copy link
Member Author

@anapaulagomes thanks for the test remember! I added it in 1ff384a

Do you feel we can merge this? If so, I'll also release a new bakery's version.

Copy link
Contributor

@anapaulagomes anapaulagomes left a comment

Choose a reason for hiding this comment

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

A tiny thing is left to change. Also, Travis must be green before merging it. :)

CHANGELOG.md Outdated Show resolved Hide resolved
model_bakery/baker.py Show resolved Hide resolved
Co-Authored-By: Ana Paula Gomes <apgomes88@gmail.com>
@berinhard
Copy link
Member Author

Great! Once the build finishes and passes, I'll merge this 👍

@berinhard berinhard merged commit 9ae2b27 into master Jan 22, 2020
@berinhard berinhard deleted the bugfix/custom-field branch January 22, 2020 14:30
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

Successfully merging this pull request may close these issues.

Ability to load custom fields for Model Mommy to also generate
2 participants