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

Change configurations.setup() to importer.install() #292

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ewelkaw
Copy link

@ewelkaw ewelkaw commented Sep 15, 2021

Trying to run example from the docs with Celery results in error message:

django.core.exceptions.ImproperlyConfigured: django-configurations settings importer wasn't correctly installed. Please use one of the starter functions to install it as mentioned in the docs: https://django-configurations.readthedocs.io/

However, changing that to:

from configurations import importer
importer.install()

works fine.

This PR updates the documentation to provide correct command.

Trying to run example from the docs with Celery results in error message:
```
django.core.exceptions.ImproperlyConfigured: django-configurations settings importer wasn't correctly installed. Please use one of the starter functions to install it as mentioned in the docs: https://django-configurations.readthedocs.io/
```

However, changing that to:
```
from configurations import importer
importer.install()
```

works fine.

This PR updates the documentation to provide correct command.
@codecov
Copy link

codecov bot commented Sep 21, 2021

Codecov Report

Merging #292 (2c39c41) into master (d89fe5a) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #292   +/-   ##
=======================================
  Coverage   89.89%   89.89%           
=======================================
  Files          26       26           
  Lines        1198     1198           
  Branches       98       98           
=======================================
  Hits         1077     1077           
  Misses         92       92           
  Partials       29       29           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d89fe5a...2c39c41. Read the comment docs.

@brianhelba
Copy link
Contributor

@ewelkaw I'm not able to reproduce this myself.

Internally, the only difference is that configurations.setup() calls django.setup() too. However, Celery also calls django.setup() internally, since doing so is required in order to use Django settings.

If there's a good reason for this change, I'd be happy to consider it, but configurations.setup() is generally the preferred high-level way to bootstrap things, so I don't think we should change the docs unless it's clearly understood why it's necessary.

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.

2 participants