Allow to replace some standard classes


(Cédric Krier) #1

Rational

Some usages need to replace some standard classes, for example this Redis Cache implementation.
It is better for everyone that users don’t need to fork Tryton to extend/customize some parts.

Proposal

The proposal is inspired by the logging.config module where it is possible to define alternative classes.
We could add similar behaviour for some classes where it makes sense to be able to replace them. The configuration could be extended to support new options. For example, we could add under the cache section an option class that will replace the trytond.cache.Cache class with this kind of pattern:

class Cache(object):
    ...

if config.get('cache', 'class'):
    from logging.config import _resolve
    Cache = _resolve(config.get('cache', 'class')) 

The possible classes to be replaced:

  • trytond.cache.Cache
  • trytond.model.*
  • trytond.model.fields.*
  • trytond.report.Report
  • trytond.wizard.*

Implemetation

cache: https://bugs.tryton.org/issue5962
filestore: https://bugs.tryton.org/issue5686


(Nicolas Évrard) #2

Indeed there is a common need to override some of the classes Tryton defines.
Moreover the importlib features of python3 might render this feature quite elegant.


(Jean Cavallo) #3

I was wondering if it would be possible to add a similar behaviour for trytond.pool.Pool

I had to do this :

because I wanted in a module to run some validation code once the pool was fully loaded.


(Cédric Krier) #4

I don’t see what kind of valid needs would require such thing.


(albert) #5

We also need that. In our case it is for BaBI module where we register “dynamic” models and need some way to initialize them.

I must say, however, that in our case, we need a way to create such “dynamic” model in one of the trytond instances and also ensure it is registered when needed in other trytond instances of the same database.


(Cédric Krier) #6

When I read such requirements, it lights up a big :warning: in my mind because this is a tendency that leads by finally coding into the application. It also raises the complexity in such way that it will generate Heisenberg bugs.
Indeed, I think the same level of flexibility could be achieved using a table_query feature.


(albert) #7

We did it this way because it was simple: trytond is in charge of creating the database structure, for example (babi materializes the report).

To achieve something similar using table_query we should probably use the context to know which report the user is querying and make fields_get (and fields_view_get) as well as table_query return the required information. But we should create the table ourselves (probably not a big issue, though).


(Jesús Martín Jiménez) #8

But, IMHO even although the BABI module of NaN·tic were using table_query feature, it is needed to setup the model with a dynamic model in order to show views and reports, isn’t it?


(Cédric Krier) #9

In some way, I don’t really care what will be needed to do. What I care about is to prevent to have bad design in Tryton. And for me, dynamic model is bad design. Tryton tries to be only declarative based because we believed it was more robust and have less side-effect (see: PYSON, states, on_change_with etc.).
So if we come back to the original request which was to be able to have a post Pool.init, I don’t see any valid usage other than having dynamic models which is against the paradigm of Tryton.


(Cédric Krier) #10

issue5686 implements this design for the FileStore class.
I think it is a good example to follow.


(Sergi Almacellas Abellana) #11

issue5962 Added the implementation for the cache backend


(Cédric Krier) #12

I think for those classes a better design should be found because usually this feature will be needed by modules. So we need a way in a module to register/replace base classes. This could be done by registering in a special register those subclasses and to force all standard classes to inherit from.


(Sergi Almacellas Abellana) #13

I think this part should be solved by issue4735


(Cédric Krier) #14

Probably but we should have a clean design and I think the extension should be conditional to being a subclass of one of the base classes.


(albert) #15

I think the same or similar mechanism could be used to override the dispatcher or allowing at least to override the way exceptions are managed. This would allow, for example, using sentry without the need to patch trytond.


(Cédric Krier) #16

For me, sentry should be managed with a specific logging Handler.


(albert) #17

The idea is not only send it to sentry (or whatever service) but send the user a different message. That is replace a standard exception by a raise_user_error with custom message.


(Cédric Krier) #18

I think this will be best managed with a configuration feature than a override/replacement.


(albert) #19

I don’t see how this could be managed with a configuration feature. For example, with sentry what you do is send a message such as “There was an application error. We’ve given tracked the error and given it the ID xxxxxxx. If you want you can phone us and give this ID as reference.”. So this ID would be given by sentry.


(Cédric Krier) #20

So the solution will need to define a generic API.