Community mailing list archives

community@mail.odoo.com

Re: broken Travis builds in OCA repositories

by
Pedro M. Baeza
- 03/25/2015 06:56:45
As stated before, you don't need any logger, because declaring extenal_dependencies on module manifest, you will get an error when trying to use the module, and you avoid the error on server startup.

You will need to install the dependency, and restart server, and then, the variable will have the dependency loaded.

Regards.

2015-03-25 11:53 GMT+01:00 Alexandre Fayolle <alexandre.fayolle@camptocamp.com>:
I'm fine with both approaches.

Quick comparison of options (I may have missed pros / cons, please fill in if you thing it's needed)


option 1: at top of module use
-----------------------------------------
try:
     import xxx;
except ImportError:
    pass

pros:
* only one import (not one attempt each time the function is called)

cons:
* if external dep is uninstalled, silent error at install, and crash when addon is used

option 2: at top of module use
------------------------------------------
try:
     import xxx;
except ImportError:
    _logger.info('module xxx not found')

pros:
* no silent exception

cons:
* you get a log message even if the addon is not actually installed. Maybe use __logger.debug.

option 3: import the external dep when needed (inside a method)
---------------------------------------------------------------------------------------

pros:
* no silent exception

cons:
* you still get a crash at runtime if dependency is removed
* runtime penalty (multiple import has a small cost)





On 25/03/2015 10:52, Mignon, Laurent wrote:
<blockquote cite="mid:CALcN+6Fa2Fp8gMiV9gSmZTNkD=VnwYUgq=_v4ir8Kd9bAjd3-Q@mail.gmail.com" type="cite">
Hi,

IMO, imports must remains on top of the module. import inside the code is not a good practice and should be discouraged.  I prefer a try catch on top of the file to a import lost in the code.

my 2 cents,

lmi

On Wed, Mar 25, 2015 at 10:43 AM, Stefan <stefan@therp.nl> wrote:
On 25-03-15 10:40, Alexandre Fayolle wrote:
> On 25/03/2015 10:17, Stefan wrote:
>
> >
> > Hi Alexandre,
> >
> > thank you for picking this up. When we encountered this in the past, our
> > solution was to import the external dependency locally in the methods
> > themselves. This preserves the clear runtime error mentioning the
> > missing dependency. Would you consider this solution instead of catching
> > ImportError at the top of the modules?
> >
> >
>
> That should be fine, yes, unless the addon is used in a tight loop. I'll
> update the existing PRs and use this approach for the next ones.

Great, thanks!

_______________________________________________
Mailing-List: https://www.odoo.com/groups/community-59
Post to: mailto:community@mail.odoo.com
Unsubscribe: https://www.odoo.com/groups?unsubscribe




--
Laurent Mignon
Senior Software Engineer

Tel : +352 20 21 10 20 32
Fax : +352 20 21 10 21
Gsm : +352 691 506 009
Email: laurent.mignon@acsone.eu

Acsone SA, Succursale de Luxembourg
22, Zone industrielle
L-8287 Kehlen, Luxembourg


_______________________________________________
Mailing-List: https://www.odoo.com/groups/community-59
Post to: mailto:community@mail.odoo.com
Unsubscribe: https://www.odoo.com/groups?unsubscribe



-- 
Alexandre Fayolle
Chef de Projet
Tel : + 33 (0)4 79 26 57 94

Camptocamp France SAS
Savoie Technolac, BP 352
73377 Le Bourget du Lac Cedex
http://www.camptocamp.com

_______________________________________________
Mailing-List: https://www.odoo.com/groups/community-59
Post to: mailto:community@mail.odoo.com
Unsubscribe: https://www.odoo.com/groups?unsubscribe