RubyMoney / eu_central_bank

A gem that calculates the exchange rate using published rates from European Central Bank. Compatible with the money gem
MIT License
217 stars 130 forks source link

Historical rates support #81

Closed bak1an closed 2 years ago

bak1an commented 6 years ago

This adds support for fetching all historical rates from http://www.ecb.europa.eu/stats/eurofxref/eurofxref-hist.xml instead of just the last 90 days in update_historical_rates method (90 days historical rates is still the default).

Also adds save_historical_rates method which calls save_rates with proper historical urls.

See the very first commit to get an idea what the actual change was, I tried to make it compatible with existing API not to change any behavior for existing applications.

The rest of commits here are to fix problems that appeared after, namely:

# saving full history to local file:
$ bundle exec ruby -e "require 'eu_central_bank'; EuCentralBank.new.save_historical_rates('/tmp/all.xml', true)" 

# dom/xpath parser, just loading the rates from local file:
$ time bundle exec ruby -e "require 'eu_central_bank'; EuCentralBank.new.update_historical_rates('/tmp/all.xml', true)" 
bundle exec ruby -e   408.07s user 5.07s system 90% cpu 7:34.00 total

# new sax parser, you can see that it is 400+ seconds vs 3 seconds issue:
$ time bundle exec ruby -e "require 'eu_central_bank'; EuCentralBank.new.update_historical_rates('/tmp/all.xml', true)"
bundle exec ruby -e   3.15s user 0.20s system 96% cpu 3.481 total

This looks ready for some initial review, will add specs for new behavior meanwhile.

Refs: #52

bak1an commented 5 years ago

@antstorm Looks like this slipped through my fingers back then. Rebased it with:

Let me know what you think.

Thanks! (and sorry for the delay)

dv commented 4 years ago

@antstorm any chance this can get merged? Seems like a nice upgrade, useful for any accounting software.

dv commented 4 years ago

@bak1an I did some rebasing in https://github.com/dv/eu_central_bank/tree/hist-rates feel free to update your branch with this.

Currently your branch does not work with an updated Money gem due to the mutex/transaction difference (getting a "cannot call synchronize on nil" error).

bak1an commented 4 years ago

Sure, can do it next week.

ghost commented 4 years ago

@bak1an pleeeeease :D

bak1an commented 4 years ago

@antstorm Rebased and it works. Had to keep jruby nokogiri workaround though, one we have in gemspec is still affected by that issue.

ghost commented 3 years ago

@bak1an @antstorm can't wait :D

ghost commented 3 years ago

?

gildesmarais commented 3 years ago

Yesterday, semmons99 closed a bunch of PRs and issues without any further comment. This one is affected, too. Am I missing an spring clean announcement or something? This PR looked fine to me and would have added a nice feature to the gem.

semmons99 commented 3 years ago

closing of long-standing PRs without resolution happy to reopen and merge is @antstorm gives the 👍

bak1an commented 2 years ago

@antstorm Any chance we can reopen and merge this? This is still useful (I even run it in production)

bak1an commented 2 years ago

Or maybe @semmons99 can point to some other maintainers that are active recently to consider reopening this

semmons99 commented 2 years ago

@bak1an give me a signal if/when the PR goes green and I will merge and release

bak1an commented 2 years ago

@semmons99 merged in main branch, need approval for CI to run

semmons99 commented 2 years ago

@bak1an going to merge into main and release now

semmons99 commented 2 years ago

@bak1an released https://rubygems.org/gems/eu_central_bank/versions/1.7.0

bak1an commented 2 years ago

@semmons99 The most awesome, thanks!