SeattleTestbed / custominstallerbuilder

Django app to customize SeattleTestbed installers with public keys
MIT License
0 stars 7 forks source link

Upgrade to at least Django 1.8 Fix - #6 #21

Closed lukpueh closed 7 years ago

lukpueh commented 7 years ago

This PR tackles all Django Deprecation Warnings I found in my log when clicking through a test instance of CustomInstallerBuilder. See commit messages for details.

I tested running python manage.py runserver and browsing localhost as well as running the test scripts in custominstallerbuilder/tests on Django 1.8, 1.9 and 1.10.

A few notes on testing: The script example_stress_test.py seems to be broken (a very brief search suggested that it might be related to threading on OS X). The scripts test_build_installers.py and test_invalid_input.py had to be monkey patched because they import integrationtestlib and send_gmail which I don't know where from. Apart from that everything worked fine on Django 1.8. On Django 1.9. and 1.10 Seattle's namespace patching (safe_type) conflicts with the development server's django setup. For 1.9 one can bypass the problem by running python manage.py runserver --no-threading. For 1.10. we have to actually fix the problem. We should also check if the problem persists when running on a development server (e.g. Apache).

lukpueh commented 7 years ago

Fixes https://github.com/SeattleTestbed/custominstallerbuilder/issues/6

lukpueh commented 7 years ago

Here is a minimal breaking example that illustrates how importing safe.py into the django app's namespace won't let me python manage.py runserver in Django 1.9 and 1.10:

>>> import types
>>> class SomeClassWhoseInstanceIWantToCheckInDjango:
...   pass
...
>>> def safe_type(*args, **kwargs):
...   pass
...
>>> __builtins__.type = safe_type
>>> my_instance = SomeClassWhoseInstanceIWantToCheckInDjango()
>>> isinstance(my_instance, (type, types.ClassType))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: isinstance() arg 2 must be a class, type, or tuple of classes and types
aaaaalbert commented 7 years ago

Maybe the patch proposed here helps. It assumes that repyportability gets imported and changes which builtins are (kept from being) replaced. SeattleTestbed/portability#35

aaaaalbert commented 7 years ago

Have an even more minimal example:

isinstance(object(), type)

Combine with this observation on when the replacement happens, and you can trigger the error.

aaaaalbert commented 7 years ago

Thanks @lukpueh, great job!

TODO: The deprecation warnings for Django 1.9/1.10 warrant a separate issue.