Implement RetryingClient

[martinnj]

Goal here is basically to help with #307.
And takes inspiration from this comment

The idea is a thin "wrapper client" that does nothing but forward calls and retry as appropriate.
The features are as follows:

• Takes any Client or Client-like class.
• Configurable number of retries.
• Configurable sleep between retries. (Don't think it's really a good idea in a cache, but it was easy, so here we are.)
• Allows retrying only for specific Exception types.
• Allows skipping retries for specific Exception types.

I haven't written tests yet since I wanted some opinions on the interface/constructor first.

Currently the class uses __getattr__ to pass through any attributes/methods that isn't present on itself to the inner client.
It also passes through dir() calls to the inner client to allow for checking available methods, attributes, etc.
Is this an acceptable implementation or is it too opaque?

I also did a more naive implementation, it is commented out in the code.

Feedback and suggestions are welcome.
I'll write the unit-tests ones the feel of the client is more solid.

@cgordon are you the person to go to for this? 😄

[cgordon]

There are a couple existing "retry" libraries on PyPI, and it is probably worth either implementing their full set of functionality, or just using them directly. Here are the two I found:

https://pypi.org/project/retry/
https://pypi.org/project/retrying/

[martinnj]

There are a couple existing "retry" libraries on PyPI, and it is probably worth either implementing their full set of functionality, or just using them directly. Here are the two I found:

https://pypi.org/project/retry/
https://pypi.org/project/retrying/

Was there any specific features from those libraries you where wanting?
Currently the timeout, retry count and exception based actions is implemented.

I didn't look for existing libraries because adding dependencies for something like this would be a shame in my eyes.

[4383]

There are a couple existing "retry" libraries on PyPI, and it is probably worth either implementing their full set of functionality, or just using them directly. Here are the two I found:

https://pypi.org/project/retry/
https://pypi.org/project/retrying/

Tenacity is an interesting implementation and already well used on openstack.

However, I agree with @martinnj , if possible, it would be more interesting to keep pymemcache as a pure python library (I voluntarly ignore six). Maybe by only providing specific retry strategies without tons of configurations. Less is more.

[4383]

Also notice that Wednesday I also started to implement similar features, however, I didn't yet proposed them before today through a pull request. #326

By introspection the passed clients, I dynamically decorate their methods to make them "retryable".

It's not a problem for me if you decide to continue with this version (that of @martinnj). Maybe we could converge our implementations to design and co-author something, it's up to you to decide. I propose you dispose.

[martinnj]

It's not a problem for me if you decide to continue with this version (that of @martinnj). Maybe we could converge our implementations to design and co-author something, it's up to you to decide. I propose you dispose.

Wild timing, I'll just wait with spending more time until a collaborator decides either way. :)

[martinnj]

I had some time so I thought I'd give it a shot anyway.

Changes:

• I had to redo the retry/passthrough logic since I had grossly misunderstood __getattr__.
• Fixed style issues.
• Fleshed out docstrings.
• Added unit tests.

I haven't added a unit-test for the "delay" function. If forced I would time how long the call took and check that it was strictly longer than the delay argument. But I also feel like inserting second long delays in the unit-tests is a bit weird.

Let me know if I should start squashing my commits so there is less bloat in the history.

EDIT: Might've send out an email notification or two too many last night. Sorry about that. Github and me are not always best friends.

[cgordon]

@martinnj I'm out of town right now and mostly away from my computer. I will have time to look through this next week, apologies for the delay.

[martinnj]

@martinnj I'm out of town right now and mostly away from my computer. I will have time to look through this next week, apologies for the delay.

No stress, it's all good.

[4383]

Hello

@martinnj: Please can you also add related doc with some examples of usage?
maybe there https://pymemcache.readthedocs.io/en/latest/getting_started.html

[4383]

Hello see my inline comment about abc

[4383]

Nice changes, thank you for applying my previous comments.

Here is a couple of new suggestions, they aren't mandatory comments. I think that they can help to improve this feature.

[4383]

Also, I think that it could be worth to reword and to squash your commits into a all in one commit to avoid to spread the history of these changes.

Example:

$ git rebase -i HEAD~12
reword ee1aa4c Add first draft for RetryingClient.
squash 9325a3c Corrections, docstrings and fixes for RetryingClient.
squash 0700983 Stop retring AttributeError and style fixes.
squash eddbe50 Stylefixes for flake8.
squash 6664775 Minor fixes and docstrings for RetryingClient.
squash cf2b44d Add unit-tests for RetryingClient.
squash f08d348 Add RetryingClient example to the getting started docuementation.
squash 71f81bb More robust import of Iterable.
squash b0782c5 0 index retry range.
squash 03d7c48 Reorder imports of tests for pop8ness.
squash eee78cc Sharpen test-case.
squash 5da4090 Simplify argument parsing.

All these changes are related to a single one feature.

[martinnj]

Also, I think that it could be worth to reword and to squash your commits into a all in one commit to avoid to spread the history of these changes.

Example:

$ git rebase -i HEAD~12
reword ee1aa4c Add first draft for RetryingClient.
squash 9325a3c Corrections, docstrings and fixes for RetryingClient.
squash 0700983 Stop retring AttributeError and style fixes.
squash eddbe50 Stylefixes for flake8.
squash 6664775 Minor fixes and docstrings for RetryingClient.
squash cf2b44d Add unit-tests for RetryingClient.
squash f08d348 Add RetryingClient example to the getting started docuementation.
squash 71f81bb More robust import of Iterable.
squash b0782c5 0 index retry range.
squash 03d7c48 Reorder imports of tests for pop8ness.
squash eee78cc Sharpen test-case.
squash 5da4090 Simplify argument parsing.

All these changes are related to a single one feature.

Plan was to squash just before merging, since the commits give context to some of the comments in the PR.

Edit:
Thanks for all the feedback btw. :)

[4383]

Also, I think that it could be worth to reword and to squash your commits into a all in one commit to avoid to spread the history of these changes.
Example:

$ git rebase -i HEAD~12
reword ee1aa4c Add first draft for RetryingClient.
squash 9325a3c Corrections, docstrings and fixes for RetryingClient.
squash 0700983 Stop retring AttributeError and style fixes.
squash eddbe50 Stylefixes for flake8.
squash 6664775 Minor fixes and docstrings for RetryingClient.
squash cf2b44d Add unit-tests for RetryingClient.
squash f08d348 Add RetryingClient example to the getting started docuementation.
squash 71f81bb More robust import of Iterable.
squash b0782c5 0 index retry range.
squash 03d7c48 Reorder imports of tests for pop8ness.
squash eee78cc Sharpen test-case.
squash 5da4090 Simplify argument parsing.

All these changes are related to a single one feature.

Plan was to squash just before merging, since the commits give context to some of the comments in the PR.

Edit:
Thanks for all the feedback btw. :)

Thanks to you too :)

No problem it make sense to squash it just before.

[martinnj]

@cgordon Hate to be pushy, but any news on this one? :)

[cgordon]

LGTM! I left one comment that you can do, or not, at your discretion.

[cgordon]

Hmm, I hit "Comment" instead of "Approve" by accident, anyone know how to navigate this UI to approve this PR?

[martinnj]

Hmm, I hit "Comment" instead of "Approve" by accident, anyone know how to navigate this UI to approve this PR?

@cgordon Good question. No clue. I've hit "Re-request review" for the blocking changes, might give you a prompt to clear it. :)