fiobank icon indicating copy to clipboard operation
fiobank copied to clipboard

Allow getting transactions with info in one request.

Open lichvarm opened this issue 11 years ago • 8 comments

This patch allows to read both transactions and the account information in one request. This is useful to avoid the server request rate limit (error 409).

lichvarm avatar Aug 28 '13 21:08 lichvarm

Thank you! However, I'd like to solve this without abandoning straightforward API. I'd like to provide clear and usable interface while abstracting the implementation. Therefore I think the way to go is to cache the first response and then return information and transaction again in separate methods, with consistent API.

One problem with this would be up-to-dateness of returned data. User expects the data he/she gets are realtime, with no cache involved. I'll think about this.

honzajavorek avatar Aug 29 '13 09:08 honzajavorek

Thanks for looking into it. I'll be happy with any solution that allows me to get both results without hitting the 30 second limit.

I thought about caching, but it seemed to me the account info and the transaction list should go together. They are both relevant to the specified period as the account info includes the closing balance and may include the opening balance. How about breaking the API and always returning the info and the list as a dict or a tuple?

lichvarm avatar Aug 29 '13 11:08 lichvarm

I thought about it and I prefer following solution:

  • tune the API little bit so it provides info together with transactions - possibly it could return something less primitive then just lists and dicts (I'd prefer enhanced subclasses of those with ability to provide more information, so the change would be nice and seamless)
  • introduce some 30s caching and handle HTTP 409, so the library returns the same results as in previous request in case 409 occures - this would be the right thing to do, because in that moment there is no way how to get more recent information than the previous one

I see only one problem with this - fast requests to different methods still cannot use caching and 409 will be thrown (I'd prefer custom exception though). At least, info would be still available in every response, lowering the number of needed requests.

honzajavorek avatar Sep 26 '13 09:09 honzajavorek

I am definitely not reloading info but I think that otherwise the api should fall loudly and clearly - with specific error and leave the caching up to the user..

Visgean avatar Feb 12 '15 15:02 Visgean

Time and @Visgean convinced me that propagation of custom exception is the right thing to do.

The interface is a different beast and there are some good points about balances etc. I could introduce parallel methods returning both or I could tune the existing API as I mentioned above. Let's see.

honzajavorek avatar Feb 12 '15 16:02 honzajavorek

Maybe something like this:


class FioBank(object):
    _info = None

    def info(self):
        if self._info:
            return self._info
        today = date.today()
        data = self._request('periods', from_date=today, to_date=today)
        self._info = self._parse_info(data)
        return _info

But there needs to be some mechanism for reloading/refreshing the info....

Visgean avatar Feb 12 '15 16:02 Visgean

It's not in my current possibilities to work on it (all my opensource focus now goes to the planned new version of https://github.com/pyvec/python.cz/), but PRs with some neat, non-bc-breaking solutions are welcome :)

honzajavorek avatar Feb 12 '15 16:02 honzajavorek

Thanks for all the work spent on this. In 2024 I think we could add a separate method which returns both info and transactions. That method would be called transactions and would be a preferred way how to use the library. The current methods would bring up deprecation warnings.

honzajavorek avatar Jun 13 '24 20:06 honzajavorek