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

Adds a required django.setup() call to unit tests. #987

Merged
merged 1 commit into from Sep 20, 2016

Conversation

mhrivnak
Copy link
Contributor

Some unit tests failed under django 1.9 without this setup call.

https://pulp.plan.io/issues/2257
fixes #2257

@mention-bot
Copy link

@mhrivnak, thanks for your PR! By analyzing the annotation information on this pull request, we identified @seandst to be a potential reviewer

@@ -9,6 +10,10 @@
devel_base.block_load_conf()


if django.VERSION >= (1, 9):
django.setup()
Copy link
Contributor

@seandst seandst Sep 16, 2016

Choose a reason for hiding this comment

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

This function has nothing specifically to do with unit tests. If it's needed for testing, it's likely needed for all of Django. Some indication about what this function is, and why we're calling it where we're calling it is needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

The Django docs say:

This function is called automatically:

    When running an HTTP server via Django’s WSGI support.
    When invoking a management command.

It must be called explicitly in other cases, for instance in plain Python scripts.

Are you not running tests with python manage.py test?

Copy link
Contributor Author

@mhrivnak mhrivnak Sep 16, 2016

Choose a reason for hiding this comment

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

Good question. I'm using the run-tests.py script. We don't have a manage.py in pulp 2.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add an in-line comment explaining why it's needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

If the unittests need it to do what they do, then presumably anything doing similar things to the unittests will also need it. Does it make more sense to put this into an initialization function rather than make the test runtime environment differ from the normal runtime environment?

Copy link
Contributor Author

@mhrivnak mhrivnak Sep 16, 2016

Choose a reason for hiding this comment

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

Another good question. The docs say that this is not necessary "when invoked by your Web server", so presumably the wsgi processes are fine as-is.

https://docs.djangoproject.com/en/1.9/topics/settings/#calling-django-setup-is-required-for-standalone-django-usage

It was hard to figure out how it gets called by workers, because I couldn't find anything in pulp's code that even imports from django during worker startup. Apparently celery itself looks for django and does "something" with it during startup, including a call to django.setup():

Sep 16 22:29:41 dev celery[4750]:   File "/usr/bin/celery", line 9, in <module>
Sep 16 22:29:41 dev celery[4750]:     load_entry_point('celery', 'console_scripts', 'celery')()
Sep 16 22:29:41 dev celery[4750]:   File "/usr/lib/python2.7/site-packages/celery/__main__.py", line 30, in main
Sep 16 22:29:41 dev celery[4750]:     main()
Sep 16 22:29:41 dev celery[4750]:   File "/usr/lib/python2.7/site-packages/celery/bin/celery.py", line 81, in main
Sep 16 22:29:41 dev celery[4750]:     cmd.execute_from_commandline(argv)
Sep 16 22:29:41 dev celery[4750]:   File "/usr/lib/python2.7/site-packages/celery/bin/celery.py", line 770, in execute_from_commandline
Sep 16 22:29:41 dev celery[4750]:     super(CeleryCommand, self).execute_from_commandline(argv)))
Sep 16 22:29:41 dev celery[4750]:   File "/usr/lib/python2.7/site-packages/celery/bin/base.py", line 311, in execute_from_commandline
Sep 16 22:29:41 dev celery[4750]:     return self.handle_argv(self.prog_name, argv[1:])
Sep 16 22:29:41 dev celery[4750]:   File "/usr/lib/python2.7/site-packages/celery/bin/celery.py", line 762, in handle_argv
Sep 16 22:29:41 dev celery[4750]:     return self.execute(command, argv)
Sep 16 22:29:41 dev celery[4750]:   File "/usr/lib/python2.7/site-packages/celery/bin/celery.py", line 694, in execute
Sep 16 22:29:41 dev celery[4750]:     ).run_from_argv(self.prog_name, argv[1:], command=argv[0])
Sep 16 22:29:41 dev celery[4750]:   File "/usr/lib/python2.7/site-packages/celery/bin/worker.py", line 179, in run_from_argv
Sep 16 22:29:41 dev celery[4750]:     return self(*args, **options)
Sep 16 22:29:41 dev celery[4750]:   File "/usr/lib/python2.7/site-packages/celery/bin/base.py", line 274, in __call__
Sep 16 22:29:41 dev celery[4750]:     ret = self.run(*args, **kwargs)
Sep 16 22:29:41 dev celery[4750]:   File "/usr/lib/python2.7/site-packages/celery/bin/worker.py", line 212, in run
Sep 16 22:29:41 dev celery[4750]:     state_db=self.node_format(state_db, hostname), **kwargs
Sep 16 22:29:41 dev celery[4750]:   File "/usr/lib/python2.7/site-packages/celery/worker/__init__.py", line 95, in __init__
Sep 16 22:29:41 dev celery[4750]:     self.app.loader.init_worker()
Sep 16 22:29:41 dev celery[4750]:   File "/usr/lib/python2.7/site-packages/celery/loaders/base.py", line 128, in init_worker
Sep 16 22:29:41 dev celery[4750]:     self.import_default_modules()
Sep 16 22:29:41 dev celery[4750]:   File "/usr/lib/python2.7/site-packages/celery/loaders/base.py", line 116, in import_default_modules
Sep 16 22:29:41 dev celery[4750]:     signals.import_modules.send(sender=self.app)
Sep 16 22:29:41 dev celery[4750]:   File "/usr/lib/python2.7/site-packages/celery/utils/dispatch/signal.py", line 166, in send
Sep 16 22:29:41 dev celery[4750]:     response = receiver(signal=self, sender=sender, **named)
Sep 16 22:29:41 dev celery[4750]:   File "/usr/lib/python2.7/site-packages/celery/fixups/django.py", line 73, in on_import_modules
Sep 16 22:29:41 dev celery[4750]:     self.worker_fixup.validate_models()
Sep 16 22:29:41 dev celery[4750]:   File "/usr/lib/python2.7/site-packages/celery/fixups/django.py", line 158, in validate_models
Sep 16 22:29:41 dev celery[4750]:     django_setup()

So, it seems that all of our normal runtime environments already make this call.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's all I could think of as well for where invocation might be needed...

  • tests runners
  • web services
  • task workers

...with the task works being the most worrisome.

@seandst seandst added the LGTM label Sep 19, 2016
Some unit tests failed under django 1.9 without this setup call.

https://pulp.plan.io/issues/2257
fixes pulp#2257
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants