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

Support BF request throttling #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Support BF request throttling #4

wants to merge 1 commit into from

Conversation

astondg
Copy link

@astondg astondg commented Mar 11, 2015

I'm submitting this pull request on behalf of Zac Sky. We have embarked on a project that covers a wide range of BF markets and requires support for BF request throttling. As such Zac has updated MarketListener to support BF request throttling when retrieving MarketBooks and added a KeepAlive method to BetfairClient.

…ng MarketBooks. Added KeepAlive method to BetfairClient.
@joelpob
Copy link
Owner

joelpob commented Mar 15, 2015

Thanks for the pull request -- I added the KeepAlive code and bug fixes in my latest push. The weighted request code is useful but I'd like to request a few changes if you have time:

  1. Batching can be done in a separate method before invoking BetfairClient.ListMarketBook. Makes it a little cleaner that way.
  2. I'm not sure we want to "Task.WhenAll()" over the batches: this implies that we're waiting for all batch http requests to finish and return, increasing the risk of one failing or taking too long. If we do (1) and return an array of Tasks for the batches this allows the caller to decide to what do in that case.
  3. If we do it for ListMarketBook, we may as well do it for ListMarketCatalogue and ListMarketProfitAndLoss (https://api.developer.betfair.com/services/webapps/docs/display/1smk3cen4v3lu3yomq5qye0ni/Market+Data+Request+Limits).

I also tweaked KeepAlive in my latest push to wrap it in a BetfairServerResponse (gives you latency and error metrics).

Thanks!

@storm-99
Copy link

great idea to get api-ng and streaming api working together. going to explore this :D
any chance for putting the nuget.config and nuget.targets so that I can build the solution?
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.

3 participants