About glue modules
Cédric Krier
With the latest developments, I started to write a lot of glue modules
like:
- purchase_shipment_cost_percentage
- sale_shipment_cost_weight
and I start to need one more purchase_shipment_cost_anglo_saxon because
the accounting move when the unit price include shipment cost must be
splitted.
All this glue modules don't add new functionality but fix existing one
because 2 (or more) modules are activated on the same database.
I think it means there is something wrong with this design.
So, I would like to know what is your feeling about having some code in
some modules that tests if an other module is activated or not? It will
be a little bit like duck-typing.
By the way, we already have some code with such design like the template
in the module product or the grouping of shipment in sale and purchase.
The main issue, I see in this way, is that some feature should need to
wait a new release to be added. For example, the purchase_shipment_cost
can not be developed for 2.2 as it will require a modification of
account_stock_anglo_saxon.
A second is about testing, it will be harder to test such part of the
code because by default the extra-module will not be installed.
--
Cédric Krier
B2CK SPRL
Rue de Rotterdam, 4
4000 Liège
Belgium
Tel: +32 472 54 46 59
Email/Jabber: cedric...@b2ck.com
Website: http://www.b2ck.com/
Albert Cervera i Areny
A Divendres, 23 de desembre de 2011 14:09:40, C�dric Krier va escriure:
> Hi,
>
> With the latest developments, I started to write a lot of glue modules
> like:
>
> - purchase_shipment_cost_percentage
> - sale_shipment_cost_weight
>
> and I start to need one more purchase_shipment_cost_anglo_saxon because
> the accounting move when the unit price include shipment cost must be
> splitted.
And probably the product_kit module will require something like this too.
> All this glue modules don't add new functionality but fix existing one
> because 2 (or more) modules are activated on the same database.
>
> I think it means there is something wrong with this design.
>
> So, I would like to know what is your feeling about having some code in
> some modules that tests if an other module is activated or not? It will
> be a little bit like duck-typing.
>
> By the way, we already have some code with such design like the template
> in the module product or the grouping of shipment in sale and purchase.
>
> The main issue, I see in this way, is that some feature should need to
> wait a new release to be added. For example, the purchase_shipment_cost
> can not be developed for 2.2 as it will require a modification of
> account_stock_anglo_saxon.
> A second is about testing, it will be harder to test such part of the
> code because by default the extra-module will not be installed.
Some other thoughts to add to the discussion:
I think we need that compatibility/dependency to be in the module definition. So if the user installs the modules "purchase_shipment_cost" and "stock_account_ango_saxon", then "purchase_shipment_cost_anglo_saxon" should be automatically marked for installation as if it was a dependency. That information should probably come from the purchase_shipment_cost_anglo_saxon module, or it may come from any of the other two.
In case of the approach of modifying the existing module, maybe the __tryton__ file should also inform that the module takes into account a given set of otherwise incompatible modules. I think we need a way to know which modules require some others or check for their existance.
As you said, if a module requires another one to be modified it may not be possible to develop it for a previous version which I think it is very bad so it sounds as the wrong solution from this point of view. Ferdinand Gassauer once suggested to be able for a module to include patches that would be applied to existing modules. I don't like that idea very much but maybe it inspires somebody for a more flexible solution.
--
Albert Cervera i Areny
Tel: +34 93 553 18 03
Cédric Krier
> I think we need that compatibility/dependency to be in the module definition.
> So if the user installs the modules "purchase_shipment_cost" and
> "stock_account_ango_saxon", then "purchase_shipment_cost_anglo_saxon" should
> be automatically marked for installation as if it was a dependency.
So both module "purchase_shipment_cost" and "stock_account_ango_saxon"
need to know each other. There is no such option in distutils and from
my Gentoo packager experience, I never saw such dependencies.
> That
> information should probably come from the purchase_shipment_cost_anglo_saxon
> module, or it may come from any of the other two.
It can not because you can have the two modules without this one.
> In case of the approach of modifying the existing module, maybe the __tryton__
> file should also inform that the module takes into account a given set of
> otherwise incompatible modules.
I think it will be just an extra_dependency in setup.py (and in
INSTALL).
> I think we need a way to know which modules
> require some others
This is already there.
> or check for their existance.
I think extra_deps is enough.
> As you said, if a module requires another one to be modified it may not be
> possible to develop it for a previous version which I think it is very bad so
> it sounds as the wrong solution from this point of view.
Not too bad as we have release every 6 months. And moreover, it is
already the case for many new modules like the groupby design change for
the sale_supply_drop_shipment.
> Ferdinand Gassauer
> once suggested to be able for a module to include patches that would be
> applied to existing modules. I don't like that idea very much but maybe it
> inspires somebody for a more flexible solution.
I don't see how it could work and even I don't see how it could fix our
issue. Patching other module is the same as extending Models.
Albert Cervera i Areny
A Divendres, 23 de desembre de 2011 17:19:51, C�dric Krier va escriure:
> On 23/12/11 16:39 +0100, Albert Cervera i Areny wrote:
> > I think we need that compatibility/dependency to be in the module
> > definition. So if the user installs the modules "purchase_shipment_cost"
> > and "stock_account_ango_saxon", then
> > "purchase_shipment_cost_anglo_saxon" should be automatically marked for
> > installation as if it was a dependency.
>
> So both module "purchase_shipment_cost" and "stock_account_ango_saxon"
> need to know each other. There is no such option in distutils and from
> my Gentoo packager experience, I never saw such dependencies.
Well, don't know about those, really. But Debian has an "incompatible" and "suggested" options. Maybe something like "suggested" could be enough. But if you allow the user to install those modules that are known to be incompatible... I don't know I think we should try to do better and suggest the module that makes the system work.
> > That
> > information should probably come from the
> > purchase_shipment_cost_anglo_saxon module, or it may come from any of
> > the other two.
>
> It can not because you can have the two modules without this one.
It depends on how you express that. It could be an in "purchase_shipment_cost_anglo_saxon" that says:
"required" if X and Y are installed.
> > In case of the approach of modifying the existing module, maybe the
> > __tryton__ file should also inform that the module takes into account a
> > given set of otherwise incompatible modules.
>
> I think it will be just an extra_dependency in setup.py (and in
> INSTALL).
>
> > I think we need a way to know which modules
> > require some others
>
> This is already there.
I meant something like the conditional I expressed above.
> > or check for their existance.
>
> I think extra_deps is enough.
Yes, that could be enough.
> > As you said, if a module requires another one to be modified it may not
> > be possible to develop it for a previous version which I think it is
> > very bad so it sounds as the wrong solution from this point of view.
>
> Not too bad as we have release every 6 months. And moreover, it is
> already the case for many new modules like the groupby design change for
> the sale_supply_drop_shipment.
I agree that the 6 month cycle helps a lot here but I think the framework should be ready for both approaches: glue modules and code module checks. The latter is ok for core modules but the former will be required for modules outside core.
So improving the way to express the kind of dependencies (with things like suggest or conditionals) between modules can become an interesting feature.
Cédric Krier
> A Divendres, 23 de desembre de 2011 17:19:51, Cédric Krier va escriure:
> > On 23/12/11 16:39 +0100, Albert Cervera i Areny wrote:
> > > I think we need that compatibility/dependency to be in the module
> > > definition. So if the user installs the modules "purchase_shipment_cost"
> > > and "stock_account_ango_saxon", then
> > > "purchase_shipment_cost_anglo_saxon" should be automatically marked for
> > > installation as if it was a dependency.
> >
> > So both module "purchase_shipment_cost" and "stock_account_ango_saxon"
> > need to know each other. There is no such option in distutils and from
> > my Gentoo packager experience, I never saw such dependencies.
>
> Well, don't know about those, really. But Debian has an "incompatible" and
> "suggested" options.
"incompatible" should never happen. If such things happen then it is all
the module that must be forbiden.
> Maybe something like "suggested" could be enough.
I find this Debian feature pointless because nobody understand what does
it mean. And more over as it is just suggested, I don't need to install
it.
> > > That
> > > information should probably come from the
> > > purchase_shipment_cost_anglo_saxon module, or it may come from any of
> > > the other two.
> >
> > It can not because you can have the two modules without this one.
>
> It depends on how you express that. It could be an in
> "purchase_shipment_cost_anglo_saxon" that says:
>
> "required" if X and Y are installed.
It still need to be on both side. If the other one was not installed
when the first is, and you install it later than you don't get the
requirement.
Any way, I don't think conditional requirement is the solution. And I
never saw any package manager having such functionality.
And I really don't want to maintain such glue modules, I want to remove
them.
Albert Cervera i Areny
A Divendres, 23 de desembre de 2011 19:09:25, C�dric Krier va escriure:
> > It depends on how you express that. It could be an in
> > "purchase_shipment_cost_anglo_saxon" that says:
> >
> > "required" if X and Y are installed.
>
> It still need to be on both side. If the other one was not installed
> when the first is, and you install it later than you don't get the
> requirement.
> Any way, I don't think conditional requirement is the solution. And I
> never saw any package manager having such functionality.
>
> And I really don't want to maintain such glue modules, I want to remove
> them.
As I said, probably core modules can manage without those modules but I'm not so sure modules outside will be able to live without them. Anyway we can wait until it actually becomes an issue for "external" modules.
Cédric Krier
> A Divendres, 23 de desembre de 2011 19:09:25, Cédric Krier va escriure:
> > > It depends on how you express that. It could be an in
> > > "purchase_shipment_cost_anglo_saxon" that says:
> > >
> > > "required" if X and Y are installed.
> >
> > It still need to be on both side. If the other one was not installed
> > when the first is, and you install it later than you don't get the
> > requirement.
> > Any way, I don't think conditional requirement is the solution. And I
> > never saw any package manager having such functionality.
> >
> > And I really don't want to maintain such glue modules, I want to remove
> > them.
>
> As I said, probably core modules can manage without those modules but I'm not
> so sure modules outside will be able to live without them. Anyway we can wait
> until it actually becomes an issue for "external" modules.
There is no issue about writing glue modules. I already did it.
Cédric Krier
First, we can have module extending Models define on other modules but
without depending on it. Like puting the code of
sale_shipment_cost_weight in carrier_weight but without making
carrier_weight depending on sale.
Second, we just need to ensure the carrier_weight will be loaded after
sale_shipment_cost if it is installed. For this, let's introduce an
other keyword "extra_depends" that will behave like "depends" but it
will not be read when installing the module.
I think this design will allow to put glue code inside the module
without crappy hack and it will be explicit via the extra_depends.
Sharoon Thomas
I think I have found a solution.
First, we can have module extending Models define on other modules but
without depending on it. Like puting the code of
sale_shipment_cost_weight in carrier_weight but without making
carrier_weight depending on sale.
Second, we just need to ensure the carrier_weight will be loaded after
sale_shipment_cost if it is installed. For this, let's introduce an
other keyword "extra_depends" that will behave like "depends" but it
will not be read when installing the module.
I think this design will allow to put glue code inside the module
without crappy hack and it will be explicit via the extra_depends.
Director & CEO, Openlabs Technologies & Consulting Pvt Limited
Mobile: +1 786 247 1317
http://openlabs.co.in
Udo Spallek
Am Sat, 24 Dec 2011 07:45:18 +0530
schrieb Sharoon Thomas <sharoo...@teagarden.in>:
> On Dec 24, 2011, at 3:09 AM, Cédric Krier wrote:
> > Second, we just need to ensure the carrier_weight will be loaded
> > after sale_shipment_cost if it is installed. For this, let's
> > introduce an other keyword "extra_depends" that will behave like
> > "depends" but it will not be read when installing the module.
> This will not prevent tryton module loader from loading classes which
> were meant to be inherited (but will be loaded as new). For example if
> you inherit `sale.sale` and change the behaviour of a method, and if
> sale module is not in `depends` tryton will create `sale.sale` as a
> new model ?
Maybe we can control the class level with a decorator, like:
@depends_module('account_invoice')
class Invoice(...):
_name='account.invoice'
...
Regards Udo
--
_____________________________
virtual things
Preisler & Spallek GbR
München - Aachen
Windeckstr. 77
81375 München
Tel: +49 (89) 710 481 55
Fax: +49 (89) 710 481 56
Mathias Behrle
+0530):
> On Dec 24, 2011, at 3:09 AM, Cédric Krier wrote:
>
> > I think I have found a solution.
> >
> > First, we can have module extending Models define on other modules but
> > without depending on it. Like puting the code of
> > sale_shipment_cost_weight in carrier_weight but without making
> > carrier_weight depending on sale.
>
> Which means no separate glue module :)
>
> > Second, we just need to ensure the carrier_weight will be loaded after
> > sale_shipment_cost if it is installed. For this, let's introduce an
> > other keyword "extra_depends" that will behave like "depends" but it
> > will not be read when installing the module.
>
> This will not prevent tryton module loader from loading classes which
> were meant to be inherited (but will be loaded as new). For example if
> you inherit `sale.sale` and change the behaviour of a method, and if
> sale module is not in `depends` tryton will create `sale.sale` as a new
> model ?
>
> >
> > I think this design will allow to put glue code inside the module
> > without crappy hack and it will be explicit via the extra_depends.
>
> I dont know if this is really a dependency management problem ? (if I
> understood the problem right. Please correct me if I am wrong)
>
> I dont think these glue modules introduce new fields in DB and they only
> overwrite existing methods or introduce new methods to change the
> functionality.
I think, glue modules could want to add new fields.
> Why not provide an efficient helper function which checks if a module is
> installed ?
>
> ```
> from trytond.tools import is_module_installed
>
> class Something(ModelSQL):
> def create(self, values):
> if is_module_installed(['sale', 'something_else']):
> # do something
> #anyway do this which is common
>
> ```
>
> Though this might make methods bulky, I think it makes it more findable (and
> hence readable) rather than having a lot of glue modules distributing
> functionality across the place.
I like the direction of this idea most until now, because something like this
could also provide the flexibility needed to handle Or cases:
if is_module_installed(['sale_kit']):
# do something
elif is_module_installed(['sale_production']):
# do soemthing else
Perhaps sort of conditional loader in __init__.py or something similar could do
the trick?
from company import *
if_module_installed('company_extension'):
from company_extension import *
--
Mathias Behrle
MBSolutions
Gilgenmatten 10 A
D-79114 Freiburg
Tel: +49(761)471023
Fax: +49(761)4770816
http://m9s.biz
UStIdNr: DE 142009020
PGP/GnuPG key availabable from any keyserver, ID: 0x8405BBF6
Cédric Krier
> On Dec 24, 2011, at 3:09 AM, Cédric Krier wrote:
>
> > I think I have found a solution.
> >
> > First, we can have module extending Models define on other modules but
> > without depending on it. Like puting the code of
> > sale_shipment_cost_weight in carrier_weight but without making
> > carrier_weight depending on sale.
>
> Which means no separate glue module :)
>
> > Second, we just need to ensure the carrier_weight will be loaded after
> > sale_shipment_cost if it is installed. For this, let's introduce an
> > other keyword "extra_depends" that will behave like "depends" but it
> > will not be read when installing the module.
>
> This will not prevent tryton module loader from loading classes which
> were meant to be inherited (but will be loaded as new). For example if
> you inherit `sale.sale` and change the behaviour of a method, and if
> sale module is not in `depends` tryton will create `sale.sale` as a new
> model ?
That's not a problem if you use Model and not ModelSQL.
Also that's why the other_depends is important to have the right order
if the module is there.
> >
> > I think this design will allow to put glue code inside the module
> > without crappy hack and it will be explicit via the extra_depends.
>
> I dont know if this is really a dependency management problem ? (if I
> understood the problem right. Please correct me if I am wrong)
>
> I dont think these glue modules introduce new fields in DB and they only
> overwrite existing methods or introduce new methods to change the functionality.
>
> Why not provide an efficient helper function which checks if a module is installed ?
>
> ```
> from trytond.tools import is_module_installed
>
> class Something(ModelSQL):
> def create(self, values):
> if is_module_installed(['sale', 'something_else']):
> # do something
> #anyway do this which is common
>
I think duck-typing is better here and more pythonic.
> Though this might make methods bulky, I think it makes it more findable (and
> hence readable) rather than having a lot of glue modules distributing functionality
> across the place.
Normally we should try to limit the glue code and such code should be
placed in a logical place.
Cédric Krier
> Hi,
> Am Sat, 24 Dec 2011 07:45:18 +0530
> schrieb Sharoon Thomas <sharoo...@teagarden.in>:
> > On Dec 24, 2011, at 3:09 AM, Cédric Krier wrote:
> [...]
> > > Second, we just need to ensure the carrier_weight will be loaded
> > > after sale_shipment_cost if it is installed. For this, let's
> > > introduce an other keyword "extra_depends" that will behave like
> > > "depends" but it will not be read when installing the module.
> > This will not prevent tryton module loader from loading classes which
> > were meant to be inherited (but will be loaded as new). For example if
> > you inherit `sale.sale` and change the behaviour of a method, and if
> > sale module is not in `depends` tryton will create `sale.sale` as a
> > new model ?
>
> Maybe we can control the class level with a decorator, like:
>
> @depends_module('account_invoice')
> class Invoice(...):
> _name='account.invoice'
> ...
Not possible because there is no database up when loading modules.
Cédric Krier
> > Why not provide an efficient helper function which checks if a module is
> > installed ?
> >
> > ```
> > from trytond.tools import is_module_installed
> >
> > class Something(ModelSQL):
> > def create(self, values):
> > if is_module_installed(['sale', 'something_else']):
> > # do something
> > #anyway do this which is common
> >
> > ```
> >
> > Though this might make methods bulky, I think it makes it more findable (and
> > hence readable) rather than having a lot of glue modules distributing
> > functionality across the place.
>
> I like the direction of this idea most until now, because something like this
> could also provide the flexibility needed to handle Or cases:
>
> if is_module_installed(['sale_kit']):
> # do something
> elif is_module_installed(['sale_production']):
> # do soemthing else
This is really something to avoid absolutly because it breaks the
modularity.
Bertrand Chenal
Cédric Krier <cedric...@b2ck.com> a écrit :
> On 24/12/11 07:45 +0530, Sharoon Thomas wrote:
> > On Dec 24, 2011, at 3:09 AM, Cédric Krier wrote:
> >
> > > I think I have found a solution.
> > >
> > > First, we can have module extending Models define on other
> > > modules but without depending on it. Like puting the code of
> > > sale_shipment_cost_weight in carrier_weight but without making
> > > carrier_weight depending on sale.
> >
> > Which means no separate glue module :)
> >
> > > Second, we just need to ensure the carrier_weight will be loaded
> > > after sale_shipment_cost if it is installed. For this, let's
> > > introduce an other keyword "extra_depends" that will behave like
> > > "depends" but it will not be read when installing the module.
> >
> > This will not prevent tryton module loader from loading classes
> > which were meant to be inherited (but will be loaded as new). For
> > example if you inherit `sale.sale` and change the behaviour of a
> > method, and if sale module is not in `depends` tryton will create
> > `sale.sale` as a new model ?
>
> That's not a problem if you use Model and not ModelSQL.
> Also that's why the other_depends is important to have the right order
> if the module is there.
I like the concept (and it looks something fun to implement!), but the
name "other_depends" feels like a real dependency, I propose "follows".
> > >
> > > I think this design will allow to put glue code inside the module
> > > without crappy hack and it will be explicit via the extra_depends.
> >
> > I dont know if this is really a dependency management problem ? (if
> > I understood the problem right. Please correct me if I am wrong)
> >
> > I dont think these glue modules introduce new fields in DB and they
> > only overwrite existing methods or introduce new methods to change
> > the functionality.
> >
> > Why not provide an efficient helper function which checks if a
> > module is installed ?
> >
> > ```
> > from trytond.tools import is_module_installed
> >
> > class Something(ModelSQL):
> > def create(self, values):
> > if is_module_installed(['sale', 'something_else']):
> > # do something
> > #anyway do this which is common
> >
>
> I think duck-typing is better here and more pythonic.
Could you provide an example of code? I'm not sure to see what is
duck-typing here, do you talk about the kind of test we do for
migration?
--
Bertrand Chenal
B2CK SPRL
Rue de Rotterdam, 4
4000 Liège
Belgium
Tel: +32 472 54 46 59
Email: bertran...@b2ck.com
Website: http://www.b2ck.com/
Cédric Krier
I choose other_depends because it is the name in distutils.
> > > >
> > > > I think this design will allow to put glue code inside the module
> > > > without crappy hack and it will be explicit via the extra_depends.
> > >
> > > I dont know if this is really a dependency management problem ? (if
> > > I understood the problem right. Please correct me if I am wrong)
> > >
> > > I dont think these glue modules introduce new fields in DB and they
> > > only overwrite existing methods or introduce new methods to change
> > > the functionality.
> > >
> > > Why not provide an efficient helper function which checks if a
> > > module is installed ?
> > >
> > > ```
> > > from trytond.tools import is_module_installed
> > >
> > > class Something(ModelSQL):
> > > def create(self, values):
> > > if is_module_installed(['sale', 'something_else']):
> > > # do something
> > > #anyway do this which is common
> > >
> >
> > I think duck-typing is better here and more pythonic.
>
> Could you provide an example of code? I'm not sure to see what is
> duck-typing here, do you talk about the kind of test we do for
> migration?
It depends of the cases but it could be like:
if hasattr(sale_obj, 'carrier'):
...
--
Cédric Krier
B2CK SPRL
Rue de Rotterdam, 4
4000 Liège
Belgium
Tel: +32 472 54 46 59