Add Google currency data provider. #481
Conversation
|
@rossigee Can you fix pylint issues ? |
|
@adrienpeiffer - I've fixed the missing line it reports in the Python file. The RST file error doesn't appear to be related to my patch, but if it is, I don't know why or how to fix. |
| # You should have received a copy of the GNU Affero General Public License | ||
| # along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
| # | ||
| ############################################################################## |
There was a problem hiding this comment.
Can you use a shorter header ?
e.g. https://github.com/OCA/maintainer-tools/blob/master/template/module/__openerp__.py
|
@rossigee I just tested your module on runbot. But why I try to update currency that takes a lot of time with no result. I tested only with one currency (GBP). Did I miss something ? |
|
@adrienpeiffer - Sorry, I'm not familiar with runbot. I have only tested this on real systems. How can I reproduce? |
|
The related runbot build is killed. I just started a new one. You have to wait the end of the build to test it |
|
@adrienpeiffer - I still do not understand. How can I reproduce the problem you are describing? |
|
You can test it here : http://3293799-481-6f94e7.runbot2.odoo-community.org/web/login |
|
@adrienpeiffer - what is the login? |
|
admin - admin 😄 |
|
@adrienpeiffer - thanks, that worked. Unfortunately, I'm having trouble configuring the instance in order to test with it. This is what I did after logging in.
At this point, regardless of whether I choose an existing webservice (i.e. Yahoo) or the new one, it does not offer me any currencies to add when I click 'Add an item' in the 'Currencies to update with this service' table. What have I missed? |
|
All currencies was inactive. I just created a record of currency.rate.update.service in runbot |
|
OK, it seems you must choose the service, click 'Save' and then add currencies. I was trying to add currencies while creating the service, which it seems unable to do. That's a separate bug/issue, as is trying to add a currency without a service being selected, which produces a JS stacktrace. Anyway. I can now see/reproduce the issue on runbot. However, I have no idea where to find runbot's instance logs to look for errors/warnings, or how to further debug the instance. As a control measure, I have also added the Yahoo service and configured it similarly. This also exhibits the same behaviour, which suggests that either: a) This is some kind of bug present in more than just the new service. Are we sure that the runbot instance is able to make outbound HTTPS calls successfully? |
There was a problem hiding this comment.
Thanks for the contribution!
Can you improve the README file:
- tags
- document title
- footer
- can you improve usage section with a procedure following comments Add Google currency data provider. #481 (comment) and Add Google currency data provider. #481 (comment)
Can you bump version number in manifest?
| if match: | ||
| val = match.group()[16:-11] | ||
| if curr in self.invert_currencies: | ||
| val = str(1.0 / float(val)) |
There was a problem hiding this comment.
I don't see why that would be necessary. The other service implementations don't check for zero. IMHO, if it were necessary, that should be the subject of a separate patch that addresses the potential issue in all the services.
|
@elicoidal - This patch does not change any documented behaviour. Such improvements/additions to the README should be the subject of a separate patch. |
|
@rossigee OK: understand your point about additional PR. Still a bump in version is necessary |
|
@elicoidal - the one in openerp.py? which of the five digits to bump?! I would guess '9.0.1.0.1'? (sorry, I'm new here!) |
|
@elicoidal - thanks for that. It says the 'y' figure should be incremented for non-breaking new features (i.e. 9.0.1.1.0). I've updated the patch accordingly. |
|
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
github-actions
bot
added
the
stale
As per subject, this patch adds a new provider based on the Google Currency Converter.
This was done for use with 9.0, and I'm just putting out there for anyone that might find it useful. I will submit a separate PR once I've updated and tested it to work against 10.0.