[Django] #29443: Unify registries in Django
Django
-------------------------------------+-------------------------------------
Reporter: Johannes | Owner: nobody
Hoppe |
Type: | Status: new
Cleanup/optimization |
Component: Utilities | Version: master
Severity: Normal | Keywords: plugin, registry,
Triage Stage: | pattern, utils
Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Django utilizes registries in various places thought the code, such as the
admin, models, template-tags and the check framework.
At the moment all those places provide their own implementation.
Considering that this is a recurring pattern I would suggest to unify the
implementation. More specifically to provide a common super class and
override or extend behavior if need be.
I would also suggest to add this to ``django.utils`` including public
documentation, because this pattern could be useful for Django projects
and 3rd party libraries as it is to the Django project itself.
Luckily I have written one before. I don't know if it is any good or even
compatible implementations in Django but I added a lot of documentation
and research. I am happy to share this here:
{{{
class Registry:
"""
A registry to store and retrieve elements.
This is an implementation of the "registry pattern" by Martin Fowler
(ISBN: 0321127420).
A registry can be used interact with a list of objects, without
knowing them.
It allows a package to define a common interface and to interact with
its implementations
without holding a reference to them.
A typical use case are plugins where a package provides a feature and
allows
other packages to extend it. In this case the super package might need
to interact
with its extensions, without knowing them.
This concept is used in places like Django's admin or template tags.
Typing a registry can help to define a common interface, that needs to
be implemented by
all extensions.
Usage example:
``mainapp/feature.py``::
from common.utils import Registry
class BasePlugin:
def get_url(self):
raise NotImplementedError()
plugins = Registry(entry_type=BasePlugin)
class MyFeature:
def get_plugin_urls(self):
return (plugin.get_url() for plugin in plugins)
``featureapp/feature.py``::
from mainapp.feature import BasePlugin
class MyPlugin(BasePlugin):
def get_url(self):
return "https://example.com"
``featureapp/app.py``::
from django.apps import AppConfig
from mainapp.feature import plugins
class FeatureApp(AppConfig):
name = 'featureapp'
def ready(self):
plugins.register(MyPlugin)
In order for a plugin to be registered, the ``register`` call needs to
be executed. A good place to do that in Django apps is in the app's
`AppConfig.ready` method.
Args:
entry_type (``type``):
The registry will only allow subclasses or instances of this
type to be registered.
unique (bool):
Whether or not elements can be registered multiple times.
Default: ``True``
"""
def __init__(self, entry_type=None, unique=True):
if entry_type is not None and not isinstance(entry_type, type):
raise TypeError('"entry_type" expects a class, but got an
instance.')
self.entry_type = entry_type
self.unique = unique
self._register = []
def __iter__(self):
# The list is copied, to avoid mutation of the registry while
iterating.
return iter(list(self._register))
def __len__(self):
return len(self._register)
def clear(self):
"""Clear all entries from the registry."""
self._register.clear()
def register(self, entry):
"""
Store entry in the registry.
Args:
entry: Entry that is to be added to the registry.
Raises:
TypeError:
If the entry type does not match the type of the registry
instance.
ValueError:
If the entry is already registered and the registry
instance is set to be unique.
"""
entry_type = entry if isinstance(entry, type) else type(entry)
if self.entry_type and not (
isinstance(entry, self.entry_type) or
issubclass(entry_type, self.entry_type)
):
raise TypeError('"entry" expects %s, but got %s' %
(self.entry_type, entry))
if self.unique and entry in self._register:
raise ValueError('"%s" is already registered.' % entry)
self._register.append(entry)
def unregister(self, entry):
"""
Remove all occurrences of ``entry`` from registry.
Args:
entry: Entry that is to be removed from the registry.
Raises:
ValueError: If entry is not registered.
"""
if entry not in self._register:
raise ValueError('"%s" is not registered.' % entry)
# If the registry is not unique, an element could be in the list
more than once.
while entry in self._register:
self._register.remove(entry)
}}}
I
--
Ticket URL: <https://code.djangoproject.com/ticket/29443>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Utilities | Version: master
Keywords: plugin, registry, | Triage Stage:
pattern, utils | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
New description:
Usage example:
``mainapp/feature.py``::
from common.utils import Registry
class BasePlugin:
def get_url(self):
raise NotImplementedError()
plugins = Registry(entry_type=BasePlugin)
class MyFeature:
``featureapp/feature.py``::
from mainapp.feature import BasePlugin
class MyPlugin(BasePlugin):
``featureapp/app.py``::
from django.apps import AppConfig
from mainapp.feature import plugins
def ready(self):
plugins.register(MyPlugin)
"""
def __len__(self):
return len(self._register)
I
--
--
Ticket URL: <https://code.djangoproject.com/ticket/29443#comment:1>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Utilities | Version: master
Keywords: plugin, registry, | Triage Stage:
pattern, utils | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
> Django utilizes registries in various places thought the code, such as
New description:
Usage example:
``mainapp/feature.py``::
from common.utils import Registry
class BasePlugin:
def get_url(self):
raise NotImplementedError()
plugins = Registry(entry_type=BasePlugin)
class MyFeature:
``featureapp/feature.py``::
from mainapp.feature import BasePlugin
class MyPlugin(BasePlugin):
``featureapp/app.py``::
from django.apps import AppConfig
from mainapp.feature import plugins
def ready(self):
plugins.register(MyPlugin)
"""
def __len__(self):
return len(self._register)
--
--
Ticket URL: <https://code.djangoproject.com/ticket/29443#comment:2>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Utilities | Version: master
Keywords: plugin, registry, | Triage Stage:
pattern, utils | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Johannes Hoppe):
I also have tests for the example, but it's written in PyTest but it's a
start:
{{{
class Plugin:
pass
class TestRegistry:
def test_init(self):
reg = Registry()
assert reg.unique is True
assert reg.entry_type is None
reg = Registry(entry_type=Plugin, unique=False)
assert reg.unique is False
assert reg.entry_type is Plugin
with pytest.raises(TypeError) as e:
Registry(entry_type=Plugin())
assert str(e.value) == '"entry_type" expects a class, but got an
instance.'
def test_iter(self):
reg = Registry()
assert reg._register is not reg.__iter__()
for i in range(5):
reg.register(i)
for i, entry in enumerate(reg):
reg.register(i + 5)
assert entry < 5
def test_len(self):
reg = Registry()
assert not reg
reg.register(1)
assert len(reg) == 1
def test_clear(self):
reg = Registry()
reg.register(1)
assert len(reg) == 1
reg.clear()
assert not reg
assert list(reg) == []
def test_register(self):
reg = Registry()
reg.register(1)
assert reg._register == [1], "1 should be in registry"
with pytest.raises(ValueError) as e:
reg.register(1)
assert str(e.value) == '"1" is already registered.'
assert reg._register.count(1) == 1, "1 is only once in the
registry"
reg = Registry(entry_type=list)
with pytest.raises(TypeError) as e:
reg.register(entry={})
assert str(e.value) == '"entry" expects <class \'list\'>, but got
{}'
with pytest.raises(TypeError) as e:
reg.register(entry=dict)
assert str(e.value) == '"entry" expects <class \'list\'>, but got
<class \'dict\'>'
def test_unregister(self):
reg = Registry()
with pytest.raises(Exception) as e:
reg.unregister(1)
assert str(e.value) == '"1" is not registered.'
assert not reg
reg.register(1)
assert len(reg) == 1
assert 1 in reg._register
reg.unregister(1)
assert not reg
assert 1 not in reg._register
reg = Registry(unique=False)
reg.register(1)
reg.register(2)
reg.register(1)
assert list(reg) == [1, 2, 1]
reg.unregister(1)
assert reg._register == [2]
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29443#comment:3>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Utilities | Version: master
Keywords: plugin, registry, | Triage Stage: Accepted
pattern, utils |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/29443#comment:4>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Utilities | Version: master
Keywords: plugin, registry, | Triage Stage: Accepted
pattern, utils |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Claude Paroz):
Interesting. Having a `@register` decorator would also be nice.
--
Ticket URL: <https://code.djangoproject.com/ticket/29443#comment:5>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Utilities | Version: master
Keywords: plugin, registry, | Triage Stage: Accepted
pattern, utils |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Florian Apolloner):
Rather than reinventing the wheel, can we use stuff like
https://github.com/pytest-dev/pluggy ?
--
Ticket URL: <https://code.djangoproject.com/ticket/29443#comment:6>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Utilities | Version: master
Keywords: plugin, registry, | Triage Stage: Accepted
pattern, utils |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Johannes Hoppe):
Hi Florian,
thanks for bringing this up. I didn't check 3rd party.
IMHO this particular implementation does too much. I goes way beyond a
simple registry. Replacing the existing code will be more difficult and
could introduce unwanted bug.
What concerns me more is adding a dependency.
If you prefer using this package, I am happy to run it by the mailing
list. I personally would avoid to use it.
Best
-Joe
--
Ticket URL: <https://code.djangoproject.com/ticket/29443#comment:7>
Django
-------------------------------------+-------------------------------------
Type: | Gomes
Cleanup/optimization | Status: assigned
Component: Utilities | Version: master
Severity: Normal | Resolution:
Keywords: plugin, registry, | Triage Stage: Accepted
pattern, utils |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Ana Paula Gomes
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/29443#comment:8>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Utilities | Version: master
Keywords: plugin, registry, | Triage Stage: Accepted
pattern, utils |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => new
* owner: Ana Paula Gomes => (none)
Comment:
This ticket requires more knowledge about the codebase (which I don't have
yet). But I would suggest using exceptions like AlreadyRegistered and
NotRegistered in AdminSite.
For those interested in taking a look at places implementing registers,
here a list of some classes:
- Library: template/library
- Apps: apps/registry.py
- CheckRegistry: core/checks/registry.py
- AdminSite: contrib/admin/sites.py
--
Ticket URL: <https://code.djangoproject.com/ticket/29443#comment:9>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Utilities | Version: master
Keywords: plugin, registry, | Triage Stage: Accepted
pattern, utils |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Claude Paroz):
I took a look to the Django own registry implementations, and struggled to
find much code that could be factorized in a common class. The structure
of the registries themselves are various: list, set, mapping, sometimes
several of them. Then the implementations differ much depending on the
internal registry structure.
Johannes, could you explore that a bit and show us some example of current
code that would benefit from a utility class?
--
Ticket URL: <https://code.djangoproject.com/ticket/29443#comment:10>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Utilities | Version: master
Keywords: plugin, registry, | Triage Stage: Accepted
pattern, utils |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Johannes Hoppe):
Hi Claude,
yes you are absolutely right. Ana and I discovered the same problem during
the springs at DjangoConEu'18.
The implementations are extremely different. Maybe a good reason to
refactor it, maybe a good reason to just leave it be.
It's definitely a very difficult tasks. Kudos to whomever solves this ;)
Best
-Joe
--
Ticket URL: <https://code.djangoproject.com/ticket/29443#comment:11>
Django
-------------------------------------+-------------------------------------
Cleanup/optimization |
Component: Utilities | Version: master
Keywords: plugin, registry, | Triage Stage: Accepted
pattern, utils |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => wontfix
Comment:
At first sight, I don't see a compelling reason to refactor the various
registry implementations, as they serve different purposes.
I'll close the ticket, but if someone comes later with some POC code that
demonstrates advantages to unify registry implementations, we'll happily
reconsider that decision.
--
Ticket URL: <https://code.djangoproject.com/ticket/29443#comment:12>