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

Retry batch requests (fixes #39) #51

Merged
merged 11 commits into from Feb 10, 2016
Merged

Retry batch requests (fixes #39) #51

merged 11 commits into from Feb 10, 2016

Conversation

leplatrem
Copy link
Contributor

  • Baby steps
  • Read and wait value of retry header
  • Choose location of code between batch client or client
  • Refactor

payload={'requests': chunk}
)
args = ('POST', self.endpoints.get('batch'))
kwargs = dict(payload={'requests': chunk})
Copy link
Member

Choose a reason for hiding this comment

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

I would think it's fine to actually pass everything in the kwargs.

@almet
Copy link
Member

almet commented Feb 4, 2016

That's a good start :)

@leplatrem leplatrem changed the title [WIP] Retry batch requests Retry batch requests (fixes #39) Feb 5, 2016
@leplatrem
Copy link
Contributor Author

@almet I wonder if that is the correct approach.

Instead of this, restricted to the batch request, I would suggest to generalize it to every method.

client = Client(..., retry=2, retry_after=20)

or:

client = Client()
client.create_bucket('fonts', retry=3)

and obviously:

client = Client(retry=10)
with client.batch('fonts', retry_after=0) as batch:
   batch.create_record(..)

And thus move the current to session instead

@almet
Copy link
Member

almet commented Feb 6, 2016

I see little value to have this defined for all the methods of the client. Configuring the session to handle this seems however a better approach.

Retry on error
--------------

When the server is throttled, under heavy load, or maintenance, it can
Copy link
Member

Choose a reason for hiding this comment

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

consider When the server is throttled (under heavy load or maintenance), it can return error responses.

@almet
Copy link
Member

almet commented Feb 9, 2016

r+wc (needs to be rebased)

auth=credentials,
retry=10)

the Kinto protocol lets the the server `define the duration in seconds between retries
Copy link
Contributor Author

Choose a reason for hiding this comment

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

typo: The (capital)

typo: double the the

@leplatrem
Copy link
Contributor Author

Merge looks ok!

r+ (with small typo)

almet added a commit that referenced this pull request Feb 10, 2016
@almet almet merged commit ab1d2e2 into master Feb 10, 2016
@almet almet deleted the 39-retry-for-batch branch February 10, 2016 12:16
@almet almet removed the in progress label Feb 10, 2016
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.

None yet

2 participants