Merge lp:~camptocamp/account-financial-tools/6.1-fix-1276998 into lp:~account-core-editors/account-financial-tools/6.1

Proposed by Yannick Vaucher @ Camptocamp on 2014-02-06
Status: Merged
Merged at revision: 108
Proposed branch: lp:~camptocamp/account-financial-tools/6.1-fix-1276998
Merge into: lp:~account-core-editors/account-financial-tools/6.1
Diff against target: 56 lines (+5/-10)
1 file modified
currency_rate_update/currency_rate_update.py (+5/-10)
To merge this branch: bzr merge lp:~camptocamp/account-financial-tools/6.1-fix-1276998
Reviewer Review Type Date Requested Status
Alexis de Lattre (community) code review and tests with openerp 6.1 2014-02-06 Approve on 2014-02-06
Alexandre Fayolle - camptocamp code review, no test 2014-02-06 Approve on 2014-02-06
Pedro Manuel Baeza code review Approve on 2014-02-06
Review via email: mp+205121@code.launchpad.net
To post a comment you must log in.
Pedro Manuel Baeza (pedro.baeza) wrote :

Why do you put self.logger?

Usual practice is to declare one variable:

logger = logging.get(self.__name__)

And then use it.

Regards.

review: Needs Fixing (code review)

I'm just using the self.logger which is instantiated in Currency_rate_update_service class.

This fix is a simple fix to restore currency_rate_update which is broken.

109. By Yannick Vaucher @ Camptocamp on 2014-02-06

[FIX] replace netsvc notifyChannel by logger.info and remove import of netsvc

Pedro Manuel Baeza (pedro.baeza) wrote :

OK then, I don't see that instantiation on MP, so my question was by that.

Regards.

review: Approve (code review)

line 55 could use self.logger.error(error_msg) (ie. change the logging level and use a more informative message)

review: Approve (code review, no test)
review: Approve (code review and tests with openerp 6.1)

I fast tracked this one and merged it a moment ago.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'currency_rate_update/currency_rate_update.py'
2--- currency_rate_update/currency_rate_update.py 2013-09-17 18:12:30 +0000
3+++ currency_rate_update/currency_rate_update.py 2014-02-06 11:45:49 +0000
4@@ -45,7 +45,7 @@
5 from osv import osv, fields
6 import time
7 from datetime import datetime, timedelta
8-import netsvc
9+import logging
10 from tools.translate import _
11
12 class Currency_rate_update_service(osv.osv):
13@@ -128,7 +128,6 @@
14 }
15
16 logger = logging.getLogger(__name__)
17- LOG_NAME = 'cron-rates'
18 MOD_NAME = 'currency_rate_update: '
19 def get_cron_id(self, cr, uid, context):
20 """return the updater cron's id. Create one if the cron does not exists """
21@@ -150,11 +149,7 @@
22 )
23 cron_id = int(cron_id[0])
24 except Exception, e :
25- self.logger.notifyChannel(
26- self.LOG_NAME,
27- netsvc.LOG_INFO,
28- 'warning cron not found one will be created'
29- )
30+ self.logger.info('warning cron not found one will be created')
31 pass # ignore if the cron is missing cause we are going to create it in db
32
33 # the cron does not exists
34@@ -189,13 +184,13 @@
35 # we fetch the main currency. The main rate should be set at 1.00
36 main_curr = comp.currency_id.name
37 for service in comp.services_to_use :
38- logger.debug("comp.services_to_use = %s" % (comp.services_to_use))
39+ self.logger.debug("comp.services_to_use = %s" % (comp.services_to_use))
40 note = service.note or ''
41 try :
42 # # we initalize the class that will handle the request
43 # # and return a dict of rate
44 getter = factory.register(service.service)
45- logger.debug("getter = %s" % (type(getter)))
46+ self.logger.debug("getter = %s" % (type(getter)))
47 curr_to_fetch = map(lambda x : x.name, service.currency_to_update)
48 res, log_info = getter.get_updated_currency(curr_to_fetch, main_curr, service.max_delta_days)
49 rate_name = time.strftime('%Y-%m-%d')
50@@ -227,7 +222,7 @@
51 except Exception, e:
52 error_msg = note + "\n%s ERROR : %s"\
53 % (datetime.strftime(datetime.today(), '%Y-%m-%d %H:%M:%S'), str(e))
54- self.logger.notifyChannel(self.LOG_NAME, netsvc.LOG_INFO, str(e))
55+ self.logger.info(str(e))
56 service.write({'note':error_msg})
57
58

Subscribers

People subscribed via source and target branches