Community mailing list archives

community@mail.odoo.com

Re: broken Travis builds in OCA repositories

by
Camptocamp France SAS, Alexandre Fayolle - Camptocamp
- 03/25/2015 06:45:58
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