Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throttling error for http conflict, fixing Issue #5 #7

Closed
wants to merge 7 commits into from

Conversation

Visgean
Copy link
Contributor

@Visgean Visgean commented Feb 12, 2015

No description provided.

@Visgean Visgean changed the title Updating this repo Throttling error for http conflict, fixing #5 Feb 12, 2015
@Visgean Visgean changed the title Throttling error for http conflict, fixing #5 Throttling error for http conflict, fixing Issue #5 Feb 12, 2015
@honzajavorek
Copy link
Owner

Thanks! Please, address my comments.

I was thinking of some way to encapsulate this (if someone is polling the API for some number, she already did the same request and the library could just return what was loaded the last time in case it was requested before the limit... sort of caching to overcome small time granularity), but after all, being transparent about this error is probably the best way how to handle it for now.

I'm going to merge this and release the new version soon.

import requests

from datetime import datetime, date
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this placement? I tend to cluster imports from std lib together

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might only my thing but I usually do imports in following way:

import ...

from libs import ... - libs like django etc.
from core import ... - project modules

Usually its nicer to look at because from ... import ... are much longer than simple imports... But overall it does not matter, its just my kind of fetish :D

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, I suppose, this is a clash of two fetishes :-( And I am the evil admin of this repo, so please obey, muhahaha! ;-)

@honzajavorek
Copy link
Owner

Also, it would be awesome if you could mention this error and the API limitation right in the README. Thanks!

@honzajavorek
Copy link
Owner

I scanned through issues and I see I discussed the rate limiting problem already in #4 (comment). May be useful for further thoughts.

@@ -50,8 +50,11 @@ Listing latest transactions:
>>> client.last(from_date='2013-03-01') # sets cursor to given date and returns following transactions
```

For further information [read code](https://github.com/honzajavorek/fiobank/blob/master/fiobank.py).
## Conflict error
[Fio api documentation](http://www.fio.cz/docs/cz/API_Bankovnictvi.pdf) (Section 8.2) states that a single token should be used only once per 30s. Otherwise a HTTP conflict will be returned and `Fiobank.ThrottlingError` will be raised.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

api -> API

@Visgean Visgean closed this Apr 11, 2016
@honzajavorek
Copy link
Owner

@Visgean Why this got closed? I know I didn't get to Fiobank recently, but I'm planning to dedicate it at least one evening to merge PRs, polish the code and release a new version.

@Visgean
Copy link
Contributor Author

Visgean commented Apr 11, 2016

I have closed all my PR that were inactive for a year. I will reopen this one because you seem to be interested.

@Visgean Visgean reopened this Apr 11, 2016
@honzajavorek
Copy link
Owner

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants