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

Allows import_from_str to import from inputs like app_name.ModelName #140

Merged
merged 9 commits into from
Jan 13, 2021
Merged

Allows import_from_str to import from inputs like app_name.ModelName #140

merged 9 commits into from
Jan 13, 2021

Conversation

cuducos
Copy link
Contributor

@cuducos cuducos commented Jan 2, 2021

If the reasoning in #139 makes sense (yep, it might not), this is a draft for the fix.

Locally I had to create tests/__init__.py to run tests, is that expected?

@anapaulagomes
Copy link
Contributor

Nice contribution!
Seems that tox hasn't installed the dependencies correctly. 🤦🏽 I've run this PR locally and it works.
Also, could you please add an entry in our CHANGELOG.md? Many thanks!

@anapaulagomes
Copy link
Contributor

Locally I had to create tests/init.py to run tests, is that expected?

I don't think so 🤔 What was happening before?

@cuducos
Copy link
Contributor Author

cuducos commented Jan 3, 2021

Seems that tox hasn't installed the dependencies correctly.

Yep. No idea what's happening. Any ideas from your side?

Also, could you please add an entry in our CHANGELOG.md?

Done : )

I don't think so 🤔 What was happening before?

If I don't add tests/__init__.py I get ModuleNotFoundError: No module named 'tests.generic'.

$ python -m pytest -k import_from_str
INTERNALERROR> Traceback (most recent call last):
INTERNALERROR>   File "/Users/cuducos/.virtualenvs/tempenv-228713489371e/lib/python3.8/site-packages/_pytest/main.py", line 236, in wrap_session
INTERNALERROR>     config._do_configure()
INTERNALERROR>   File "/Users/cuducos/.virtualenvs/tempenv-228713489371e/lib/python3.8/site-packages/_pytest/config/__init__.py", line 911, in _do_configure
INTERNALERROR>     self.hook.pytest_configure.call_historic(kwargs=dict(config=self))
INTERNALERROR>   File "/Users/cuducos/.virtualenvs/tempenv-228713489371e/lib/python3.8/site-packages/pluggy/hooks.py", line 308, in call_historic
INTERNALERROR>     res = self._hookexec(self, self.get_hookimpls(), kwargs)
INTERNALERROR>   File "/Users/cuducos/.virtualenvs/tempenv-228713489371e/lib/python3.8/site-packages/pluggy/manager.py", line 93, in _hookexec
INTERNALERROR>     return self._inner_hookexec(hook, methods, kwargs)
INTERNALERROR>   File "/Users/cuducos/.virtualenvs/tempenv-228713489371e/lib/python3.8/site-packages/pluggy/manager.py", line 84, in <lambda>
INTERNALERROR>     self._inner_hookexec = lambda hook, methods, kwargs: hook.multicall(
INTERNALERROR>   File "/Users/cuducos/.virtualenvs/tempenv-228713489371e/lib/python3.8/site-packages/pluggy/callers.py", line 208, in _multicall
INTERNALERROR>     return outcome.get_result()
INTERNALERROR>   File "/Users/cuducos/.virtualenvs/tempenv-228713489371e/lib/python3.8/site-packages/pluggy/callers.py", line 80, in get_result
INTERNALERROR>     raise ex[1].with_traceback(ex[2])
INTERNALERROR>   File "/Users/cuducos/.virtualenvs/tempenv-228713489371e/lib/python3.8/site-packages/pluggy/callers.py", line 187, in _multicall
INTERNALERROR>     res = hook_impl.function(*args)
INTERNALERROR>   File "/Users/cuducos/src/github.com/cuducos/model_bakery/tests/conftest.py", line 76, in pytest_configure
INTERNALERROR>     baker.generators.add("tests.generic.fields.CustomFieldViaSettings", gen_same_text)
INTERNALERROR>   File "/Users/cuducos/src/github.com/cuducos/model_bakery/model_bakery/generators.py", line 199, in add
INTERNALERROR>     user_mapping[import_from_str(field)] = import_from_str(func)
INTERNALERROR>   File "/Users/cuducos/src/github.com/cuducos/model_bakery/model_bakery/utils.py", line 28, in import_from_str
INTERNALERROR>     module = importlib.import_module(path)
INTERNALERROR>   File "/Users/cuducos/.pyenv/versions/3.8.6/lib/python3.8/importlib/__init__.py", line 127, in import_module
INTERNALERROR>     return _bootstrap._gcd_import(name[level:], package, level)
INTERNALERROR>   File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
INTERNALERROR>   File "<frozen importlib._bootstrap>", line 991, in _find_and_load
INTERNALERROR>   File "<frozen importlib._bootstrap>", line 961, in _find_and_load_unlocked
INTERNALERROR>   File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
INTERNALERROR>   File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
INTERNALERROR>   File "<frozen importlib._bootstrap>", line 991, in _find_and_load
INTERNALERROR>   File "<frozen importlib._bootstrap>", line 973, in _find_and_load_unlocked
INTERNALERROR> ModuleNotFoundError: No module named 'tests.generic'

Copy link
Member

@berinhard berinhard left a comment

Choose a reason for hiding this comment

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

Loved this work @cuducos! I hope I was able to give you the proper explanation and instructions on how to fix CI.

model_bakery/utils.py Outdated Show resolved Hide resolved
model_bakery/utils.py Outdated Show resolved Hide resolved
@cuducos
Copy link
Contributor Author

cuducos commented Jan 4, 2021

I hope I was able to give you the proper explanation and instructions on how to fix CI.

Sure you did! Makes sense.
Gonna get back to that point ASAP : )
Many thanks!

@cuducos
Copy link
Contributor Author

cuducos commented Jan 8, 2021

Ok, I suggest something different: setting apps to None when the Django import fails. Maybe it's a bit more verbose, but I think this creates room for us to better document why we cannot import external libraries in this particular module. Also, I think it makes the import_from_str a bit more clear. What do you think?

@cuducos
Copy link
Contributor Author

cuducos commented Jan 8, 2021

I have experienced the same error when using recipes, and I intend to send another commit here to allow "app_name.receipe_name" to properly import the Recipe object in the make_recipe calls, is that ok?

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
model_bakery/utils.py Outdated Show resolved Hide resolved
tests/test_recipes.py Outdated Show resolved Hide resolved
tests/test_recipes.py Show resolved Hide resolved
tests/test_recipes.py Show resolved Hide resolved
berinhard and others added 5 commits January 11, 2021 08:25
Co-authored-by: Ana Paula Gomes <[email protected]>
Co-authored-by: Ana Paula Gomes <[email protected]>
Co-authored-by: Ana Paula Gomes <[email protected]>
Co-authored-by: Ana Paula Gomes <[email protected]>
Co-authored-by: Ana Paula Gomes <[email protected]>
Copy link
Member

@berinhard berinhard left a comment

Choose a reason for hiding this comment

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

@cuducos I approved a few typos comments from @anapaulagomes but I left a last one for you to review.

model_bakery/utils.py Outdated Show resolved Hide resolved
tests/test_recipes.py Outdated Show resolved Hide resolved
Copy link
Member

@berinhard berinhard left a comment

Choose a reason for hiding this comment

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

Thanks a lot @cuducos!

@berinhard berinhard merged commit 1cd8021 into model-bakers:main Jan 13, 2021
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.

3 participants