Tryton - Issues

 

Issue6351

Title The model code is relying tests on the backend's name which renders them less overridable
Priority feature Status in-progress
Superseder Nosy List ced, nicoe, pokoli, reviewbot
Type feature request Components
Assigned To nicoe Keywords review
Reviews 33951002
View: 33951002

Created on 2017-03-10.19:01:38 by nicoe, last changed by reviewbot.

Messages
review33951002 updated at https://codereview.tryton.org/33951002/#ps180001
review33951002 updated at https://codereview.tryton.org/33951002/#ps160001
review33951002 updated at https://codereview.tryton.org/33951002/#ps140001
review33951002 updated at https://codereview.tryton.org/33951002/#ps120001
review33951002 updated at https://codereview.tryton.org/33951002/#ps80001
review33951002 updated at https://codereview.tryton.org/33951002/#ps60001
msg32520 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2017-03-15.15:32:41
As example of backend use without having a field is when you want to make CAST in a query for internal usage and so you do not have a field instance.
msg32519 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2017-03-15.14:55:05
On 2017-03-15 14:38, Nicolas Évrard wrote:
> >The communication between fields and backend should use a
> >generic/standard protocol.
> 
> In fact the only reason the field is passed is because there might be
> other case then char where we would like to have additional
> information before returning the SQL type used by the backend.
> 
> But I think that we could indeed use another attribute than _type
> something like _sql_type which would be, as you propose, the most
> generic SQL type.

I think it is wrong to have the back-end deciding how to store a field.
It is only the field that must decide. The type string should contain
all the information as it does when creating a column. It is up to the
back-end if it needs to change the type for its custom one to properly
manage extra information like size.

> >But as SQL has not real standard types definition, we must define one
> >(better to use one close to PotgreSQL).
> 
> I agree as long as we keep the design of the field that pass itself to
> the backend in order to receive the final SQL type. Which I think
> allows to delegate the responsibility of handling the type management
> where it is known: in the backend.

Wrong, it is not the role of the back-end.
msg32518 (view) Author: [hidden] (nicoe) (Tryton committer) (Tryton translator) Date: 2017-03-15.14:38:26
* Cédric Krier  [2017-03-15 11:20 +0100]: 

>The proposed desing of set 2 does not allow to create a new type of
>field without having to modify all backends. 

Indeed.

>The communication between fields and backend should use a
>generic/standard protocol.

In fact the only reason the field is passed is because there might be
other case then char where we would like to have additional
information before returning the SQL type used by the backend.

But I think that we could indeed use another attribute than _type
something like _sql_type which would be, as you propose, the most
generic SQL type.

>But as SQL has not real standard types definition, we must define one
>(better to use one close to PotgreSQL).

I agree as long as we keep the design of the field that pass itself to
the backend in order to receive the final SQL type. Which I think
allows to delegate the responsibility of handling the type management
where it is known: in the backend.
msg32515 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2017-03-15.11:20:45
I'm moving the discussion here instead of codereview because it is not related to code implementation.

The proposed desing of set 2 does not allow to create a new type of field without having to modify all backends. The communication between fields and backend should use a generic/standard protocol. But as SQL has not real standard types definition, we must define one (better to use one close to PotgreSQL).
review33951002 updated at https://codereview.tryton.org/33951002/#ps40001
New review33951002 at https://codereview.tryton.org/33951002/#ps1
msg32418 (view) Author: [hidden] (nicoe) (Tryton committer) (Tryton translator) Date: 2017-03-10.19:01:37
Thanks to the mechanism of entry point we can create new backends in separate python packages. Unfortunately if those backend inherits from a backend packaged in tryton (let's say postgis which inherits from postgresql) the developer is confronted to a lot of work because there are tests used the fields that use the backend code.

A solution to this issue is to make the fields the more backend-agnostic that is possible by transfering the responsibility of specifying the correct sql type or the specific coercition methods on the backend.
History
Date User Action Args
2017-03-23 20:33:14reviewbotsetmessages: + msg32727
2017-03-22 16:32:07reviewbotsetmessages: + msg32685
2017-03-22 16:01:34reviewbotsetmessages: + msg32683
2017-03-17 14:07:37reviewbotsetmessages: + msg32584
2017-03-17 12:35:59reviewbotsetmessages: + msg32581
2017-03-16 15:35:45reviewbotsetmessages: + msg32544
2017-03-15 15:32:41cedsetmessages: + msg32520
2017-03-15 14:55:06cedsetmessages: + msg32519
2017-03-15 14:38:26nicoesetmessages: + msg32518
2017-03-15 11:20:45cedsetnosy: + ced
messages: + msg32515

Showing 10 items. Show all history (warning: this could be VERY long)