Issue: Proposal for plugins support
Author: foxx
State: closed
Created: 2015-12-30 10:54:40

There is currently no easy way to build third party plugins within the same namespace as `peewee`, aside from monkeypatching.

Projects such as [pytest](https://pytest.org/latest/plugins.html) allow plugins to be registered into the Peewee namespace, giving a much cleaner syntax, as seen [here](https://github.com/kissgyorgy/pytest-raisesregexp), for example;

``` python
from pytest_raisesregexp import raisesregexp # without plugins
from pytest import raises_regexp # with plugins
```

This is apparently achieved via `entry_points` as seen [here](http://pytest.org/2.2.4/plugins.html#making-your-plugin-installable-by-others), and allows the plugin to interact via hooks, as seen [here](http://pytest.org/2.2.4/plugins.html#well-specified-hooks). In particular, the plugin can access `pytest_namespace` which allows manipulation of the namespace.

The use case in this situation would be for providing third party plugins which are not suitable for the core or playhouse, whilst keeping everything in a single namespace.

``` python
from peewee_dbmanager import DBManager # without plugins
from peewee.dbmanager import DBManager # with plugins OR
from peewee import DBManager # with plugins
```

This could also open the door for potentially cleaning up Playhouse, allowing each of the `extensions` to be segregated into it's own plugin, keeping the core repo clean and lean.

Thoughts @coleifer ?


+1!

> This could also open the door for potentially cleaning up Playhouse, allowing each of the extensions to be segregated into it's own plugin, keeping the core repo clean and lean.

Would be a really really nice move :)


Also, see http://www.devork.be/talks/pluggy/pluggy.html about pluggy.py (which lacks proper doc), the plugin facility of pytest.


I'm not familiar with the setup.py magic, but that's definitely pretty cool. I recall Flask used to namespace extensions under `flask_ext` using some kind of magic, but then they moved away from it. From a personal standpoint, I'm not sure what it gets you beyond having your `import` statement slightly different. Am I missing something?


In my particular use case, the primary benefits are nicer import statements and easier monkeypatching.

I've got a collection of approx 15 extensions/plugins for Peewee that I'd like to open source, but some of them require a monkeypatch entry point, meaning the user has to do this;

``` python
from peewee_dbmanager import DBManager, monkeypatch
monkeypatch()
```

Instead of just this;

``` python
from peewee.dbmanager import DBManager
```

Or even just this;

``` python
from peewee import DBManager
```

Depending on how many of the additional libraries are being used, all of a sudden a single line import turns into 20 lines of boilerplate code.

I've had a look at `pluggy` and the `setup.py` magic, it's quite awesome indeed but my primary concern is that this proposal would be rejected due to additional complexity, at the same time any plugin enhancement patch should cover future use cases, making `pluggy` the best candidate thus far due to maturity from `pytest`.


> my primary concern is that this proposal would be rejected due to additional complexity

Indeed. I'm not sure having what amounts to "vanity" imports would justify adding this additional complexity to peewee. Going to pass for now.


I think that the "vanity" imports are not only less important but might be even counter productive. I mean, when reviewing someone's code, it's not immediately clear where this import comes from (is it from peewee itself? some other library?).

But on the other hand, hooks to allow extending peewee to avoid monkey patching are very important. There are many improvements that can be offered as plugins, but shouldn't go into core, that are hard to implement or maintain without proper hooks.

Just today I monkey patched `Query` class, to be able to measure query execution time: https://github.com/getredash/redash/blob/feature/permissions/redash/metrics/database.py#L36-L62

I've also [extended the `PostgresExtDatabase` class](https://github.com/getredash/redash/blob/feature/permissions/redash/metrics/database.py#L12-L33) for similar purposes. I would preferred to offer this as a library for all `Database` derived classes, but I'm not sure if it's possible without monkey patching without proper hooks.


If the primary reason for rejection is complexity of `pluggy`, then we can achieve the same goal with just 10 lines of code added to the core, by using `entry_points` as seen in the thorough example [here](http://stackoverflow.com/questions/774824/explain-python-entry-points). This would simply look for any advertised objects, which should be callable, on `peewee_plugins` and then call them. 

This would give us simple plugins with monkeypatching support, whilst also keeping the implementation clean and simple. Thoughts @coleifer / @arikfr ?


@coleifer Has this been re-opened for consideration? If so, is there's anything I can do to help? Should I send a PR for POC?


I reopened the issue because it seems that there may be some way to add hooks to avoid 3rd party packages needing to monkey-patch. I'd like to hear some use-cases that you all are currently considering, and that would require monkey-patching, so we can come up with a better solution.


Well, I haven't really heard any use-cases where monkey-patching is currently necessary but a plugin approach would help avoid this... Please chime in if you think of something.


There is the metrics tracking use case I had:
https://github.com/getredash/redash/blob/feature/permissions/redash/metrics/database.py#L38-L64

Or being able to override `execute_sql` of the base `Database` class: 
https://github.com/getredash/redash/blob/feature/permissions/redash/metrics/database.py#L18-L29

I would love to generalize these two into a plugin.


In the first case extending via subclass + override seems correct. In the second case I'd hardly call that a common use-case...


I've tried subclassing first -- I've subclassed `Model` and changed all the query methods to return wrapped query object, but then it was lost when `.clone` was called on the query object. Obviously I can override it too, but it becomes more an more complex, and I ended up monkey patching the methods on the base class.

On the other hand if there were `before_execute` and `after_execute` hooks, I could implement it easily without overriding or monkey patching.

The second case is part of the same thing I tried to accomplish -- measure query execution time and number of queries executed per web request. And the same thing here: simple before/after hooks on execute would be enough.


For my use case, creating a database manager for Peewee requires monkeypatching to override `_execute` and `Node`, similar to @arikfr, but as he mentioned if Peewee had a hooks system then monkeypatching would not be necessary at all.


After some testing and consideration, I'm not convinced that `entry_points` is the correct approach for monkeypatch hooks. If this approach was used, then installing and importing a plugin would cause monkeypatching to happen behind the scenes, breaking the mantra of "explicit is better than implicit", meaning it's cleaner to call `someplugin.monkeypatch()`.

The problem in mine and @arikfr's use case is the inability to change Peewee internals without modifying global namespace, and a lack of hooks such as `before_execute` and `after_execute`. However even if we had those hooks, they would still impact global namespace due to the design of Peewee. Although we can subclass `Query` with the `_execute()` modifications, then apply this by subclassing `Model` and overloading `select()`, doing so could cause future problems because the entire `select()` class needs to be duplicated and modified.

One approach would be assigning class attributes to `Model` and `Database`, which are then used to determine the target class used. This approach could be quite tedious however, resulting in a lot of boilerplate on each class. For example;

``` python
class Model(with_metaclass(BaseModel)):
    select_query_class = SelectQuery

    @classmethod
    def select(cls, *selection):
        query = self.select_query_class(cls, *selection)

```

Instead it might be cleaner to use a single method which a subclass can implement;

``` python
class Model(with_metaclass(BaseModel)):
    @classmethod
    def get_class(cls):
        return cls

    @classmethod
    def select(cls, *selection):
        query = self.get_class(SelectQuery)(cls, *selection)

class MySelectQuery(SelectQuery):
    def _execute(self, *args, **kwargs):
        print("do something")
        super()._execute(*args, **kwargs)

class MyModel(Model):
    @classmethod
    def get_class(cls):
        if cls == SelectQuery:
            return MySelectQuery
        return cls
```

The above would allow us to easily patch Peewee internals on a per instance basis, without resorting to monkeypatching or peewee core duplication.

I've done some trials of implementing this into Peewee and the results so far have been fantastic, very little change in performance and no monkeypatching required.

Assuming this was accepted, I would probably be -0 on adding plugins support for now, as I wouldn't have a use case for it any longer.

Thoughts @arikfr / @coleifer ?


Urgh, yeah I'm definitely not loving it.


@coleifer This is pretty much the cleanest implementation I can think of, and after trying it some more, I've almost come to the conclusion that this isn't particularly clean from a core lib standpoint either.

Another option is module inheritance [overloading](http://stackoverflow.com/questions/7483022/clone-a-module-and-make-changes-to-the-copy), although this isn't always guaranteed to work is classes have been referenced in the original import, and therefore wouldn't be advisable.

The only clear path I'm seeing is sadly monkeypatching `peewee` module on import, and using something like [this](https://gist.github.com/foxx/87e44e00901ebddb5f93) to apply late subclasses. This has been the approach I've used since day 1, and has served me well for 6 months so far.

Sadly this does mean that my vote in this issue would be "wontfix", on the grounds that there doesn't appear to be a better alternative than monkeypatching.

Instead of a code modification, perhaps we could include a docs update which explains how to do monkeypatching and extending, along with an example, using the tools shown in the above mentioned [gist](https://gist.github.com/foxx/87e44e00901ebddb5f93). Thoughts?


+1 for avoiding entry points for the implicitness of it (and the need to publish plugins as Python packages).

How about starting with a set of simple hooks (like the before/after execute), that are global to peewee in the context of a single codebase/application? 

The implementation for the above should be straightforward and still allow many interesting extensions. If @coleifer is cool with that, I can make pull request with a suggestion for implementation.


@arikfr Could you give an example of how these hooks might be implemented and exposed? Every option I've explored has resulted with unnecessary complexity in the core, for a feature that could be easily implemented with monkey patching. Having `pre_execute` and `post_execute` can be easily done with;

``` python
class Query(peewee.Query):
    def _execute(self, *args, **kwargs):
       print("pre")
       r = super()._execute(*args, **kwargs)
       print("post")
       return r

peewee.Query = Query
```

From what I can see, the only benefit of extending Peewee internals to support hooks would be if they were on a per instance basis, however due to the design of Peewee this cannot be easily achieved (for reasons mentioned earlier, least not without a lot of redesign which results in something even more convoluted).

That being said, let me know if I've overlooked anything and I'll happily reconsider/review.


Thought I'd share my discoveries from #825, as getting monkeypatching to work consistently has been troublesome.

There is an outstanding [bug](http://bugs.python.org/issue672115) in Python which prevents dynamic modification of `__bases__`, meaning that classes which are an immediate subclass of `object` cannot be extended, where as classes which are a distant subclass can. For example;

``` python
class ModelOptions(object): pass # this cannot be extended in-line
class SqliteDatabase(Database): pass # this can
```

For classes that cannot be modified inline, the alternative approach is to overwrite classes in peewee namespace, before anything else has a chance to access them. However this is not always guaranteed to work, as seen here;

``` python
import peewee

class ModifiedSqliteDatabase(peewee.SqliteDatabase):
    pass

peewee.SqliteDatabase = ModifiedSqliteDatabase
peewee.SqliteDatabase(':memory:')
```

``` python
  File "peewee-2.8.0-py3.4.egg/peewee.py", line 3606, in __init__
  File "peewee-2.8.0-py3.4.egg/peewee.py", line 3606, in __init__
  File "peewee-2.8.0-py3.4.egg/peewee.py", line 3606, in __init__
RuntimeError: maximum recursion depth exceeded
```

This also means putting your `monkeypatch()` call before anything else, something you cannot guarantee if your application is loaded by another tool, such as py.test. Any objects created prior to the monkeypatch will use the old class, hence why this "late" approach is not preferred.

Other approaches include using a custom metaclass, such as subclassing `BaseModel` for `Model`, however this will break other libraries using the same hacks, due to [multiple inheritance inconsistencies](http://stackoverflow.com/questions/3003053/metaclass-multiple-inheritance-inconsistency).

And finally, you can manipulate an existing class, rather than applying a mixin subclass. Naturally you'll lose the ability to do `issubclass()` or `isinstance()` comparisons against your mixin, but chances are you will not need this anyway.

``` py
database = property(getter, setter, del)
peewee.ModelOptions.database = database
```

tl;dr - Different objects will require different monkeypatching approaches, and hopefully the above will save you a couple of hours.


Does the custom base ModelOptions commit in #825 (e773c620de3d7dc686dab3c2325e02d1e86407d6) address some of the concerns brought up here?


Looks like #825 resolves every issue I had with implementing a database manager for Peewee. 

There are still some situations where monkeypatching cannot be avoid, as adding certain hooks would add significant mess/complexity into the core, as seen in earlier discussions.

I'd be happy to contribute a docs patch explaining some of the monkeypatching tricks above, if you feel it would be beneficial, although it can be argued that this is a topic outside of Peewee.

Other than a possible docs patch, I'd +1 closing this issue.


Oh there is something that comes to mind, relating specifically to the example from @arikfr.

In his use case, he was overloading `Query._execute()` in order to measure execution times. Doing this by overloading `Model.select()` isn't really an option, because it would require copy/paste of Peewee internals, as mentioned earlier.

For his situation, a similar patch might also help with those cases;

``` python
class A(Model):
    class Meta:
        select_query_class = CustomSelectQuery
        raw_query_class = CustomRawQuery
        ..etc etc
```

This is the same approach as the `ModelOptions` patch, but it does result in more mess as these classes are referenced in several places. From an implementation standpoint, it would look like this;

``` python
    @classmethod
    def select(cls, *selection):
        query = self._meta.select_query_class(cls, *selection)
        if cls._meta.order_by:
            query = query.order_by(*cls._meta.order_by)
        return query
```

That being said, this would handle a significant range of plugin functionality without the need for monkeypaching. It also prevents peewee namespace from being modified, which is a huge benefit.

If this was in the core, the only instance I can think of where monkeypatching would be needed would be hotfixing internals.

Assuming @coleifer was happy with the additional mess caused, I would certainly be +1


Ehh, I'm not quite sure again why you couldn't just override `select`. Is it that common of an occurance that you need a custom `SelectQuery` implementation?


Actually I just realised something, you can override `insert()` without having to copy a bunch of internals as seen;

``` python
@classmethod
    def insert(cls, __data=None, **insert):
        fdict = __data or {} # needs copying
        fdict.update([(cls._meta.fields[f], insert[f]) for f in insert]) # needs copying
        return MyInsertQueryHere(cls, fdict) # custom lines here
```

Because the `Query()` is an instance, it becomes super easy to extend `__bases__` with none of the horrible monkeypatching bugs I mentioned earlier. For example;

``` python
@classmethod
    def insert(cls, __data=None, **insert):
        r = super(Model, cls).insert(data, **insert)
        extend_instance(r, InsertQueryMixin) # see below for gist
        return r
```

You can find my version of  `extend_instance` [here](https://gist.github.com/foxx/5807755d92a76970dae6), it's fully doc tested and been used in prod with no problems.

I can't think of any scenario that the above won't cover, including the use case mentioned by @arikfr. 

If anyone can't make their plugin work using the variety of solutions mentioned above and in this thread, then please let me know and I'll do my best to help.

Thanks for the feedback @coleifer !


Thanks for the comments, hopefully folks will be able to find this thread if they encounter similar issues.


> Is it that common of an occurrence that you need a custom SelectQuery implementation?

I also have a custom SelectQuery, and being able to just change a Meta option to use it instead of overriding `select` (including some internals copy-pasting) would be really awesome.
So I really appreciate the suggestion of @foxx exposed in [this comment](#issuecomment-173238786) (actually I was to suggest the same approach because I'm already [using](https://github.com/etalab/ban/blob/master/ban/db/model.py#L22) it).

@coleifer would you consider a PR for this?


@yohanboniface You can actually achieve this behaviour by using late mixins via [`extend_instance`](https://gist.github.com/foxx/5807755d92a76970dae6). 

This function injects other classes into an instance `__bases__`, allowing for only that specific instance to be affected by the patch. Although it might feel a bit strange relying on a non standard function, this is a tried and tested technique for monkeypatching and has proven very reliable. I've actually included it as part of a helpers library I've [released](https://github.com/foxx/bottlecap/blob/master/helpful.py), but it's not yet documented.

``` python
class SelectQueryMixin(peewee.SelectQuery):
    def _get_result_wrapper(self):
        super_func = super(SelectQueryMixin, self)._get_result_wrapper
        return getattr(self, '_result_wrapper', None) or super_func()

    def __len__(self):
        return self.count()


class Model(peewee.Model):
    class Meta:
        database = default

    @classmethod
    def select(cls, *args, **kwargs):
        obj = super(Model, cls).select(*args, **kwargs)
        extend_instance(obj, SelectQueryMixin)
        return obj
```