Summary

Update the extensions code to be compatible with Django 1.6 and higher.

Review Request #8707 — Created Feb. 2, 2017 and submitted Feb. 6, 2017, 2:31 p.m.

Information

Owner

chipx86

Repository

Djblets

Branch

release-0.10.x

Bugs

Depends On

Commit

d9f2d73...

Reviewers

Groups

djblets

People

Description

The extensions code, for many reasons, was not compatible with newer
versions of Django. We have to do a lot of manipulation of Django state
(template-related caches, model caches, installed apps, etc.) for
extensions, and this has all changed since Django 1.6.

This change adds support for manipulating all this in newer versions of
Django, and retains support for Django 1.6.

Along the way, some problems with how we were doing things were found.
Due probably to a bad merge at some point, much of the extension
initialization code was living in the media installation code and was
being called too late in the cycle. This didn't manifest as an obvious
problem on Django 1.6 (though it likely did result in apps not being
properly removed when an extension was disabled), but it was a major
problem on newer versions (which track apps and state more closely).
This code has been cleaned up and ordering has been fixed.

Models weren't being properly cleared from caches before, which wasn't a
big problem in practice but could in theory lead to
introspection-related problems. These caches are now properly cleared,
where possible.

We also now call the evolve command from Django Evolution more
correctly, ensuring compatibility not just with various versions of
Django but also with changes to the evolve command and its argument
list.

Unit tests have been added to check for all the cache-related changes,
model synchronization, and evolution operations.

Testing Done

Unit tests pass on Django 1.6 and 1.8 through 1.10.

Issues

Description	From	Last Updated
'Evolution' imported but unused	reviewbot	Feb. 2, 2017, 4:39 p.m.
'import_module' imported but unused	reviewbot	Feb. 2, 2017, 4:39 p.m.
'Evolution' imported but unused	reviewbot	Feb. 2, 2017, 4:39 p.m.
"Add" "setting wrapper" ?	brennie	Feb. 3, 2017, 2:12 p.m.
"setting wrapper" ?	brennie	Feb. 3, 2017, 2:13 p.m.
likewise, here?	brennie	Feb. 3, 2017, 2:14 p.m.
while here, docstring?	brennie	Feb. 3, 2017, 2:14 p.m.
Maybe put this into the else block so that it matches the return True? Or assign self.ref_counts[item] == 1 to …	david	Feb. 6, 2017, 1:27 p.m.

Tool: Pyflakes
Processed Files:
    djblets/extensions/test/evolve_tests/evolutions/__init__.py
    djblets/extensions/test/evolve_tests/models.py
    djblets/extensions/test/evolve_tests/evolutions/add_new_field.py
    djblets/extensions/manager.py
    djblets/extensions/test/templatetag_tests/templatetags/templatetag_tests.py
    djblets/extensions/tests.py
    djblets/extensions/test/model_tests/models.py

Ignored Files:
    djblets/extensions/test/evolve_tests/__init__.py
    djblets/extensions/test/templatetag_tests/templatetags/__init__.py
    djblets/extensions/test/templatetag_tests/__init__.py
    djblets/extensions/test/model_tests/__init__.py



Tool: PEP8 Style Checker
Processed Files:
    djblets/extensions/test/evolve_tests/evolutions/__init__.py
    djblets/extensions/test/evolve_tests/models.py
    djblets/extensions/test/evolve_tests/evolutions/add_new_field.py
    djblets/extensions/manager.py
    djblets/extensions/test/templatetag_tests/templatetags/templatetag_tests.py
    djblets/extensions/tests.py
    djblets/extensions/test/model_tests/models.py

Ignored Files:
    djblets/extensions/test/evolve_tests/__init__.py
    djblets/extensions/test/templatetag_tests/templatetags/__init__.py
    djblets/extensions/test/templatetag_tests/__init__.py
    djblets/extensions/test/model_tests/__init__.py

djblets/extensions/manager.py (Diff revision 1)
The issue has been resolved. Show all issues
```
 'Evolution' imported but unused
```
djblets/extensions/tests.py (Diff revision 1)
The issue has been resolved. Show all issues
```
 'import_module' imported but unused
```
djblets/extensions/tests.py (Diff revision 1)
The issue has been resolved. Show all issues
```
 'Evolution' imported but unused
```

Change Summary:


Django Evolution is no longer a hard requirement for projects using extensions.
Fixed some unused imports.

Commit:

2b23ff250b320c7a31510e45f45e025f7d9c051e

40f08d999bd9caa490c1a9414437cdffb327f375

Diff:

Revision 2 (+496 -145)

Show changes

	djblets/extensions/manager.py
	djblets/extensions/tests.py
	djblets/extensions/test/evolve_tests/__init__.py
	djblets/extensions/test/evolve_tests/models.py
	djblets/extensions/test/evolve_tests/evolutions/__init__.py
	djblets/extensions/test/evolve_tests/evolutions/add_new_field.py
	djblets/extensions/test/model_tests/__init__.py
	djblets/extensions/test/model_tests/models.py
	3 more

Tool: Pyflakes
Processed Files:
    djblets/extensions/test/evolve_tests/evolutions/__init__.py
    djblets/extensions/test/evolve_tests/models.py
    djblets/extensions/test/evolve_tests/evolutions/add_new_field.py
    djblets/extensions/manager.py
    djblets/extensions/test/templatetag_tests/templatetags/templatetag_tests.py
    djblets/extensions/tests.py
    djblets/extensions/test/model_tests/models.py

Ignored Files:
    djblets/extensions/test/evolve_tests/__init__.py
    djblets/extensions/test/templatetag_tests/templatetags/__init__.py
    djblets/extensions/test/templatetag_tests/__init__.py
    djblets/extensions/test/model_tests/__init__.py



Tool: PEP8 Style Checker
Processed Files:
    djblets/extensions/test/evolve_tests/evolutions/__init__.py
    djblets/extensions/test/evolve_tests/models.py
    djblets/extensions/test/evolve_tests/evolutions/add_new_field.py
    djblets/extensions/manager.py
    djblets/extensions/test/templatetag_tests/templatetags/templatetag_tests.py
    djblets/extensions/tests.py
    djblets/extensions/test/model_tests/models.py

Ignored Files:
    djblets/extensions/test/evolve_tests/__init__.py
    djblets/extensions/test/templatetag_tests/templatetags/__init__.py
    djblets/extensions/test/templatetag_tests/__init__.py
    djblets/extensions/test/model_tests/__init__.py

Hold off on this. I hit some regressions.

djblets/extensions/manager.py (Diff revision 2)
The issue has been resolved. Show all issues
```
"Add"

"setting wrapper" ?
```
1. chipx86
  
  Feb. 3, 2017, 2:14 p.m.
  It's actually adding to the setting (if not already in there).
djblets/extensions/manager.py (Diff revision 2)
The issue has been dropped. Show all issues
```
"setting wrapper" ?
```
djblets/extensions/manager.py (Diff revision 2)
The issue has been resolved. Show all issues
```
likewise, here?
```
djblets/extensions/manager.py (Diff revision 2)
The issue has been resolved. Show all issues
```
while here, docstring?
```

Change Summary:


Added/refined some docstrings.
Reworked the model clearing logic:
Some caches are now untouched, as is sys.modules. We need to leave the model modules around long-term or things can break. Some of this may be revisited down the road, particularly for Django > 1.6.
Fixed a problem where an app label without models (like rbpowerpack.hostingsvcs) could conflict with an app label with models (reviewboard.hostingsvcs) and wipe out the model registrations when removed.

Description:

		The extensions code, for many reasons, was not compatible with newer
		versions of Django. We have to do a lot of manipulation of Django state
		(template-related caches, model caches, installed apps, etc.) for
		extensions, and this has all changed since Django 1.6.

		This change adds support for manipulating all this in newer versions of
		Django, and retains support for Django 1.6.

		Along the way, some problems with how we were doing things were found.
		Due probably to a bad merge at some point, much of the extension
		initialization code was living in the media installation code and was
		being called too late in the cycle. This didn't manifest as an obvious
		problem on Django 1.6 (though it likely did result in apps not being
		properly removed when an extension was disabled), but it was a major
		problem on newer versions (which track apps and state more closely).
		This code has been cleaned up and ordering has been fixed.

		Models weren't being properly cleared from caches before, which wasn't a
		big problem in practice but could in theory lead to
~		introspection-related problems. These caches are now properly cleared.
	~	introspection-related problems. These caches are now properly cleared,
	+	where possible.

		We also now call the evolve command from Django Evolution more
		correctly, ensuring compatibility not just with various versions of
		Django but also with changes to the evolve command and its argument
		list.

		Unit tests have been added to check for all the cache-related changes,
		model synchronization, and evolution operations.

Commit:

40f08d999bd9caa490c1a9414437cdffb327f375

67ab4f629bb1f29978f64c4053b193e48c0f957c

Diff:

Revision 3 (+498 -146)

Show changes

	djblets/extensions/manager.py
	djblets/extensions/tests.py
	djblets/extensions/test/evolve_tests/__init__.py
	djblets/extensions/test/evolve_tests/models.py
	djblets/extensions/test/evolve_tests/evolutions/__init__.py
	djblets/extensions/test/evolve_tests/evolutions/add_new_field.py
	djblets/extensions/test/model_tests/__init__.py
	djblets/extensions/test/model_tests/models.py
	3 more

Tool: Pyflakes
Processed Files:
    djblets/extensions/test/evolve_tests/evolutions/__init__.py
    djblets/extensions/test/evolve_tests/models.py
    djblets/extensions/test/evolve_tests/evolutions/add_new_field.py
    djblets/extensions/manager.py
    djblets/extensions/test/templatetag_tests/templatetags/templatetag_tests.py
    djblets/extensions/tests.py
    djblets/extensions/test/model_tests/models.py

Ignored Files:
    djblets/extensions/test/evolve_tests/__init__.py
    djblets/extensions/test/templatetag_tests/templatetags/__init__.py
    djblets/extensions/test/templatetag_tests/__init__.py
    djblets/extensions/test/model_tests/__init__.py



Tool: PEP8 Style Checker
Processed Files:
    djblets/extensions/test/evolve_tests/evolutions/__init__.py
    djblets/extensions/test/evolve_tests/models.py
    djblets/extensions/test/evolve_tests/evolutions/add_new_field.py
    djblets/extensions/manager.py
    djblets/extensions/test/templatetag_tests/templatetags/templatetag_tests.py
    djblets/extensions/tests.py
    djblets/extensions/test/model_tests/models.py

Ignored Files:
    djblets/extensions/test/evolve_tests/__init__.py
    djblets/extensions/test/templatetag_tests/templatetags/__init__.py
    djblets/extensions/test/templatetag_tests/__init__.py
    djblets/extensions/test/model_tests/__init__.py

Ship it!

```
Whew!
```
1. chipx86
  
  Feb. 6, 2017, 1:27 p.m.
  I second that.

djblets/extensions/manager.py (Diff revision 3)

The issue has been resolved. Show all issues

Maybe put this into the else block so that it matches the return True? Or assign self.ref_counts[item] == 1 to a variable and then test against it, and then return it?

chipx86

Feb. 6, 2017, 1:27 p.m.
```
I'll do the former and add some docs.
```

Change Summary:

Added some docs and improved a return statement's position in a function.

Commit:

67ab4f629bb1f29978f64c4053b193e48c0f957c

d9f2d73dd6ca6b19996cebf80f66ce4accb78df8

Diff:

Revision 4 (+502 -146)

Show changes

	djblets/extensions/manager.py
	djblets/extensions/tests.py
	djblets/extensions/test/evolve_tests/__init__.py
	djblets/extensions/test/evolve_tests/models.py
	djblets/extensions/test/evolve_tests/evolutions/__init__.py
	djblets/extensions/test/evolve_tests/evolutions/add_new_field.py
	djblets/extensions/test/model_tests/__init__.py
	djblets/extensions/test/model_tests/models.py
	3 more

Tool: Pyflakes
Processed Files:
    djblets/extensions/test/evolve_tests/evolutions/__init__.py
    djblets/extensions/test/evolve_tests/models.py
    djblets/extensions/test/evolve_tests/evolutions/add_new_field.py
    djblets/extensions/manager.py
    djblets/extensions/test/templatetag_tests/templatetags/templatetag_tests.py
    djblets/extensions/tests.py
    djblets/extensions/test/model_tests/models.py

Ignored Files:
    djblets/extensions/test/evolve_tests/__init__.py
    djblets/extensions/test/templatetag_tests/templatetags/__init__.py
    djblets/extensions/test/templatetag_tests/__init__.py
    djblets/extensions/test/model_tests/__init__.py



Tool: PEP8 Style Checker
Processed Files:
    djblets/extensions/test/evolve_tests/evolutions/__init__.py
    djblets/extensions/test/evolve_tests/models.py
    djblets/extensions/test/evolve_tests/evolutions/add_new_field.py
    djblets/extensions/manager.py
    djblets/extensions/test/templatetag_tests/templatetags/templatetag_tests.py
    djblets/extensions/tests.py
    djblets/extensions/test/model_tests/models.py

Ignored Files:
    djblets/extensions/test/evolve_tests/__init__.py
    djblets/extensions/test/templatetag_tests/templatetags/__init__.py
    djblets/extensions/test/templatetag_tests/__init__.py
    djblets/extensions/test/model_tests/__init__.py

Status:: Completed
Change Summary:: Pushed to release-0.10.x (94a5793)