exactonline icon indicating copy to clipboard operation
exactonline copied to clipboard

Added bulk api support and beta calls support

Open gvisniuc opened this issue 6 years ago • 2 comments

  • Added support for bulk API calls
  • Added support for beta API calls

Example usage: beta - api.restv1(GET('financial/GLAccountClassificationMappings'), beta=True) bulk - api.restv1(GET('Financial/TransactionLines'), bulk=True)

If you are routinely pulling all of the transactions, the bulk option greatly increases the speed of it since it fetches 1k per call.

gvisniuc avatar Nov 07 '18 14:11 gvisniuc

Thanks for the PR!

So what are you saying? With the bulk API you don't need the iteration_limit option anymore?

Further: if x is True should be written as if x.

Additionally, this lacks documentation and a logical interface: if beta and bulk are true, then only bulk is used.

  • The bulk option looks clean enough to move to a separate BulkRest subclass which could be put in the right place in the API MRO. (It has nothing to do with the V1Division.)
  • But, what happens with the bulk option if you do something other than GET. Does it fail? Does it behave normally? Does it take different arguments?
  • As for the beta option: I cannot guess what it does, and as it's beta, the interface will possibly not be stable. So I'm very reluctant to add that. You can just Mix in your own ExactApi class if you really need beta in the mean time. (Replace V1Division with MyCustomBetaV1Division.)

wdoekes avatar Nov 07 '18 16:11 wdoekes

@wdoekes Hi,

Yes, that's true but it only works for a limited number of requests that have bulk support (Financial ones mostly).

Regarding the if x is True I wanted to explicitly check for the boolean True just to avoid setting the param to a string or other values that might evaluate as True.

  • Yes that sounds like a good option
  • It will most likely fail since BULK requests are GET only
  • Same, as 1, we can have a different class for that

gvisniuc avatar Nov 08 '18 08:11 gvisniuc