Merge lp:~oerp.ca/account-financial-tools/add-bank-of-canada-rss into lp:~account-core-editors/account-financial-tools/7.0

[Daniel Dico (oerp.ca)]

Add Bank of Canada to curency_rate_update

Add Bank of Canada to currency_rate_update.

Suggest to add the new log "note" at the top of the log in order to allow users to see the most recent note without having to scroll down.

As this module becomes (more) international, I suggest to remove references to ECB in check_rate_date exception and log.

[Pedro Manuel Baeza (pedro.baeza)]

Hi, Daniel, thank you very much for the contribution.

What worries me with this new source is that you are using feedparser library. This library is not detected on module install, and it will only fail if you select this source, causing the background process to fail. The alternative is to put this library as dependency, but maybe is to force user to install another library if they are not going to use this source.

What everybody thinks about this?

Regards.

review: Needs Fixing

[Daniel Dico (oerp.ca) (oerp)]

Hi Pedro,

Thank you for your review.

In regards with feedparser, I was under the impression that this library is required by OpenERP. Please see /openobject-server/openerpcommand/setup.py. I hope I'm not missing something here!?

From what I see, currency_rate_update in order to address the specific needs from a global perspective, will accumulate over time a number of different bank feeds (each with their own specific format) and, most likely, a potential user will only use one of the bank feeds... sure we can add a required library to the dependency... but it could be that users in Europe will not use the Canadian rates, while they will be forced to install the dependency. Maybe a different design could be used? Let's wait for other developers to add feeds from other banks so we can see what formats are required all over the world.

Best regards,
Daniel

> Hi, Daniel, thank you very much for the contribution.
>
> What worries me with this new source is that you are using feedparser library.
> This library is not detected on module install, and it will only fail if you
> select this source, causing the background process to fail. The alternative is
> to put this library as dependency, but maybe is to force user to install
> another library if they are not going to use this source.
>
> What everybody thinks about this?
>
> Regards.

[Pedro Manuel Baeza (pedro.baeza)]

Hi, Daniel, indeed, that library is a dependency of OpenERP itself, so it's not a problem in this version. In the future, this can change, so I think it's better to have declare python dependency on the module to prevent future problems. What do you think?

Regards.

[Daniel Dico (oerp.ca) (oerp)]

Hi Pedro,

Not sure what's best... if you look at the current code almost every implementation of the "Curreny_getter_interface" has various dependencies, and similar with feedparser they are required by OpenERP, but they are not listed as dependencies.

I've checked where feedparser is used in (7 and trunk) and there is only one place: /openobject-addons/email_template/html2text.py can this really go away? I would't know... probably not!?

It's up to you and the other reviewers... I can add it if you want me to... it's just one line of code.

Regards,
:Daniel

[Pedro Manuel Baeza (pedro.baeza)]

OK, let's stay as is for the moment and see what others think.

Thank you very much for your work and feedback!

Regards

review: Approve

[Stefan Rijnhart (Opener) (stefan-opener)]

Works fine, thanks! I agree with the other changes (reference to a specific service in a generic log msg, and have the most recent log on top).

About the feedparser dependency: while it is a dependency in OpenERP's setup.py, it is not a core library for OpenERP and may drop away in future versions. This may be easily missed during the upgrade of this module at that time. In fact, it is only used to guess encodings of emails in a piece of foreign code (by the late Aaron Schwartz no less). And in this code, a missing feedparser is caught by ImportError with a default to 'utf-8' so you can actually totally get away with running OpenERP without feedparser. But because like what has been said, only a part of the users of this module will check this particular service so you don't want to force an additional dependency on the other users. Fortunately, an ImportError because of feedparser will not crash the OpenERP server in this case, because of this module's strategy when running a service. instead, the error is logged in the update log for the user to see, which is perfectly acceptible.

review: Approve

=== modified file 'currency_rate_update/currency_rate_update.py'
--- currency_rate_update/currency_rate_update.py	2013-12-13 19:33:25 +0000
+++ currency_rate_update/currency_rate_update.py	2014-02-01 08:13:17 +0000
@@ -59,6 +59,9 @@
                                                     ('Yahoo_getter','Yahoo Finance '),
                                                     ('PL_NBP_getter','Narodowy Bank Polski'),  # Added for polish rates
                                                     ('Banxico_getter', 'Banco de México'),  # Added for mexican rates
+                                                    # Bank of Canada is using RSS-CB http://www.cbwiki.net/wiki/index.php/Specification_1.1 :
+                                                    # This RSS format is used by other national banks (Thailand, Malaysia, Mexico...)
+                                                    ('CA_BOC_getter','Bank of Canada - noon rates'),  # Added for canadian rates
                                                     ],
                                                     "Webservice to use",
                                                     required = True
@@ -211,13 +214,16 @@
                                             vals,
                                         )
 
-                    note = note + "\n%s currency updated. "\
-                       %(datetime.strftime(datetime.today(), '%Y-%m-%d %H:%M:%S'))
-                    note = note + (log_info or '')
+                    # show the most recent note at the top
+                    note = "\n%s currency updated. "\
+                       %(datetime.strftime(datetime.today(), '%Y-%m-%d %H:%M:%S'))\
+                       + note
+                    note = (log_info or '') + note
                     service.write({'note':note})
                 except Exception, e:
-                    error_msg = note + "\n%s ERROR : %s"\
-                        %(datetime.strftime(datetime.today(), '%Y-%m-%d %H:%M:%S'), str(e))
+                    error_msg = "\n%s ERROR : %s"\
+                        %(datetime.strftime(datetime.today(), '%Y-%m-%d %H:%M:%S'), str(e))\
+                        + note
                     _logger.info(str(e))
                     service.write({'note':error_msg})
 
@@ -263,6 +269,7 @@
                           'Google_getter',
                           'Yahoo_getter',
                           'Banxico_getter',
+                          'CA_BOC_getter',
                     ]
         if class_name in allowed:
             class_def = eval(class_name)
@@ -340,12 +347,12 @@
         """Check date constrains. WARN : rate_date must be of datetime type"""
         days_delta = (datetime.today() - rate_date).days
         if days_delta > max_delta_days:
-            raise Exception('The rate date from ECB (%s) is %d days away from today, which is over the limit (%d days). Rate not updated in OpenERP.'%(rate_date, days_delta, max_delta_days))
+            raise Exception('The rate timestamp (%s) is %d days away from today, which is over the limit (%d days). Rate not updated in OpenERP.'%(rate_date, days_delta, max_delta_days))
         # We always have a warning when rate_date <> today
         rate_date_str = datetime.strftime(rate_date, '%Y-%m-%d')
         if rate_date_str != datetime.strftime(datetime.today(), '%Y-%m-%d'):
-            self.log_info = "WARNING : the rate date from ECB (%s) is not today's date" % rate_date_str
-            _logger.warning("the rate date from ECB (%s) is not today's date", rate_date_str)
+            self.log_info = "WARNING : the rate timestamp (%s) is not today's date" % rate_date_str
+            _logger.warning("the rate timestamp (%s) is not today's date", rate_date_str)
 
 
 #Yahoo ###################################################################################
@@ -595,3 +602,64 @@
 
             self.updated_currency[curr] = rate
             logger.debug("Rate retrieved : " + main_currency + ' = ' + str(rate) + ' ' + curr)
+
+
+##CA BOC #####   Bank of Canada   ############################################################
+class CA_BOC_getter(Curreny_getter_interface) :
+    """Implementation of Curreny_getter_factory interface for Bank of Canada RSS service"""
+
+    def get_updated_currency(self, currency_array, main_currency, max_delta_days):
+        """implementation of abstract method of Curreny_getter_interface"""
+
+        # as of Jan 2014 BOC is publishing noon rates for about 60 currencies
+        url = 'http://www.bankofcanada.ca/stats/assets/rates_rss/noon/en_%s.xml'
+        # closing rates are available as well (please note there are only 12
+        # currencies reported):
+        # http://www.bankofcanada.ca/stats/assets/rates_rss/closing/en_%s.xml
+
+        #we do not want to update the main currency
+        if main_currency in currency_array:
+            currency_array.remove(main_currency)
+
+        import feedparser
+        import pytz
+        from dateutil import parser
+
+        for curr in currency_array:
+
+            _logger.debug("BOC currency rate service : connecting...")
+            dom = feedparser.parse(url % curr)
+
+            self.validate_cur(curr)
+
+            # check if BOC service is running
+            if dom.bozo and dom.status <> 404:
+                _logger.error("Bank of Canada - service is down - try again\
+                    later...")
+
+            # check if BOC sent a valid response for this currency
+            if dom.status != 200:
+                _logger.error("Exchange data for %s is not reported by Bank\
+                    of Canada." % curr)
+                raise osv.except_osv('Error !', 'Exchange data for %s is not\
+                    reported by Bank of Canada.' % str(curr))
+
+            _logger.debug("BOC sent a valid RSS file for: " + curr)
+
+            # check for valid exchange data
+            if (dom.entries[0].cb_basecurrency == main_currency) and \
+                    (dom.entries[0].cb_targetcurrency == curr):
+                rate = dom.entries[0].cb_exchangerate.split('\n', 1)[0]
+                rate_date_datetime = parser.parse(dom.entries[0].updated)\
+                    .astimezone(pytz.utc).replace(tzinfo=None)
+                self.check_rate_date(rate_date_datetime, max_delta_days)
+                self.updated_currency[curr] = rate
+                _logger.debug("BOC Rate retrieved : 1 " + main_currency +
+                    ' = ' + str(rate) + ' ' + curr)
+            else:
+                _logger.error("Exchange data format error for Bank of Canada -\
+                    %s. Please check provider data format and/or source code." % curr)
+                raise osv.except_osv('Error !', 'Exchange data format error for\
+                    Bank of Canada - %s !' % str(curr))
+
+        return self.updated_currency, self.log_info