New FI support: Gemini crypto exchange #45
Conversation
|
@carljm, would like a review from you please. This PR makes sense before the other one, as it fixes a mypy issue with failing tests. |
|
Appreciate you adding a new downloader @Zburatorul ! Hopefully @carljm can review and merge. |
There was a problem hiding this comment.
Seems OK to me, a few minor comments. In terms of the functionality, if I were using this I would prefer it to store dated output files and never touch or overwrite previously-downloaded files (and ideally detect date of last download, if any, and not always re-download entire history.) But I don't think this behavior is a prerequisite for merge.
finance_dl/gemini.py
Outdated
| Interactive shell: | ||
| ================== | ||
|
|
||
| From the interactive shell, type: `self.run()` to start the scraper. |
There was a problem hiding this comment.
Your PR description says something about interactive mode not being supported, that seems inconsistent with this? I'm not sure what that means, though, as far as I know scrapers don't need to do anything special to support interactive mode. Maybe I'm misunderstanding what you meant by not supporting interactive mode.
There was a problem hiding this comment.
They do require more work. You need to pop a Python interpreter for the user. I'll just mark it as not implemented and leave it for another day.
finance_dl/gemini.py
Outdated
|
|
||
| # Merge and Date | ||
| for bal in balances: | ||
| bal['price'] = bal['price'] = prices[bal['currency']]\ |
There was a problem hiding this comment.
No reason to duplicate bal['price'] = twice here
There was a problem hiding this comment.
This duplication is still present
Zburatorul
force-pushed
the
zbr-010-gemini
branch
from
84e9308 to
6590c27
Compare
|
I observed issues with rate limiting, so I had some fun wrapping the requests in some retrying logic. |
Zburatorul
force-pushed
the
zbr-010-gemini
branch
from
6590c27 to
d56fb3c
Compare
| time.sleep(wait) | ||
| try: | ||
| r = requests.get(url) if get else requests.post(url, headers=headers) | ||
| self.last_req_ts = time.time() | ||
| r.raise_for_status() | ||
| return r.json() | ||
| except requests.HTTPError as exc: | ||
| if exc.response.status_code not in self.RETRY_CODES: | ||
| raise exc |
There was a problem hiding this comment.
I think this method could be refactored to avoid duplicating this block of code (if nothing else, by extracting it to a separate method), but I don't feel strongly about it.
finance_dl/gemini.py
Outdated
|
|
||
| # Merge and Date | ||
| for bal in balances: | ||
| bal['price'] = bal['price'] = prices[bal['currency']]\ |
There was a problem hiding this comment.
This duplication is still present
Zburatorul
force-pushed
the
zbr-010-gemini
branch
from
d56fb3c to
1fb704b
Compare
Adds ability to download trades, transfers and balances from Gemini.
Interactive mode not implemented, just runs regular path.
Also had to modify
mypy.inito get tests to complete.