Skip to content

Support for retry event hook#50

Open
bodenr wants to merge 6 commits into
rholder:masterfrom
bodenr:retry_log_hook
Open

Support for retry event hook#50
bodenr wants to merge 6 commits into
rholder:masterfrom
bodenr:retry_log_hook

Conversation

@bodenr

@bodenr bodenr commented May 27, 2016

Copy link
Copy Markdown

It's very common for consumers to perform some action on each
attempt iteration; for example logging a message that the operation
failed and a retry will be performed after some time. While this can
be done today using a custom wait_func, it's inconvenient to use
a partial to wrap an existing retrying sleep function just to get this
behavior.

This patch adds support for a new kwarg called wait_event_func
that when passed should reference a function to be called
before sleeping for another attempt iteration. This function
looks similar to retrying wait_funcs except its first arg is
the time to sleep as returned by the current 'wait' function.

A handful of unit tests are also included.

bodenr and others added 3 commits May 27, 2016 09:22
It's very common for consumers to perform some action on each
attempt iteration; for example logging a message that the operation
failed and a retry will be performed after some time. While this can
be done today using a custom wait_func, it's inconvenient to use
a partial to wrap an existing retrying sleep function just to get this
behavior.

This patch adds support for a new kwarg called wait_event_func
that when passed should reference a function to be called
before sleeping for another attempt iteration. This function
looks similar to retrying wait_funcs except its first arg is
the time to sleep as returned by the current 'wait' function.

A handful of unit tests are also included.
It's very common for consumers to perform some action on each
attempt iteration; for example logging a message that the operation
failed and a retry will be performed after some time. While this can
be done today using a custom wait_func, it's inconvenient to use
a partial to wrap an existing retrying sleep function just to get this
behavior.

This patch adds support for a new kwarg called wait_event_func
that when passed should reference a function to be called
before sleeping for another attempt iteration. This function
looks similar to retrying wait_funcs except its first arg is
the time to sleep as returned by the current 'wait' function.

A handful of unit tests are also included.
No assertIsNotNone() in py26 so change to a py26 compatible assert statement.
@bodenr

bodenr commented Jun 7, 2016

Copy link
Copy Markdown
Author

@jd @harlowja @rholder
I realize this PR has conflicts now, but I'd like to understand your initial impressions of this code to determine if it's worth my time continuing here.
Thanks

@jd

jd commented Jun 9, 2016

Copy link
Copy Markdown
Contributor

LGTM, feel free to rebase!

…nto retry_log_hook

Conflicts:
	test_retrying.py
@bodenr

bodenr commented Jun 9, 2016

Copy link
Copy Markdown
Author

travis didn't rebuild; let me try closing + reopening this

@bodenr bodenr closed this Jun 9, 2016
@bodenr bodenr reopened this Jun 9, 2016
Conflicts:
	AUTHORS.rst
	retrying.py
	test_retrying.py
@bodenr

bodenr commented Jun 9, 2016

Copy link
Copy Markdown
Author

@jd looks like this one is clean now.
My apologies for all the noise here; I took a bumpy path on a git adventure.

@harlowja

Copy link
Copy Markdown
Contributor

Seems pretty useful to me 👍

@bodenr

bodenr commented Jun 16, 2016

Copy link
Copy Markdown
Author

@jd @rholder what do you guys think; is this merge-worthy?

@jd

jd commented Jun 16, 2016

Copy link
Copy Markdown
Contributor

Looks redundant with wait_func, no?

@bodenr

bodenr commented Jun 16, 2016

Copy link
Copy Markdown
Author

@jd perhaps.

As indicated in the "commit message" its very common to want a "callback" per retry (for something like logging a message). While this could be done with a custom wait_func it inconvenient to decorate and provide a custom wait_func just for this purpose. That said the intent here is to simplify that consumption pattern.

@jd

jd commented Jun 16, 2016

Copy link
Copy Markdown
Contributor

I think we don't want to clutter more the init func. What about providing that functionality as part of a custom wait_func that would be provided by retrying itself?

Possibly a composable class.

@harlowja

Copy link
Copy Markdown
Contributor

I agree with jd (also why I did #55 to help move toward a model that hopefully can be more composeable and user-friendly); so perhaps we should get #55 in first?

@bodenr

bodenr commented Jun 16, 2016

Copy link
Copy Markdown
Author

@jd @harlowja
I just pushed a commit here with some changes I was playing with that introduce function chaining using the CallChain class I whipped up (no UTs; more of a talking point).

Using this approach it's easy to just pass my "hook function" as the wait_func to achieve what I need. It also address the "chaining" TODO in the code if I understand properly.

I haven't see #55 yet, so perhaps we can weight pros/cons of it vs. the CallChain I just committed to this PR.

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