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

Add RetryingCaller and AsyncRetryingCaller #56

Merged
merged 3 commits into from Jan 28, 2024
Merged

Add RetryingCaller and AsyncRetryingCaller #56

merged 3 commits into from Jan 28, 2024

Conversation

hynek
Copy link
Owner

@hynek hynek commented Jan 28, 2024

Implements #45

Please lmk if it does what you need @RonnyPfannschmidt.

Copy link

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

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

Great 👍

@hynek hynek merged commit b5314a2 into main Jan 28, 2024
19 checks passed
@hynek hynek deleted the retrying-caller branch January 28, 2024 10:22
@hynek
Copy link
Owner Author

hynek commented Jan 28, 2024

On a second look, and before I release anything – wouldn't it make more sense if on would be the first argument to __call__?

I.e.:

rc = RetryingCaller()
rc(httpx.HTTPError, func, kw=42)

You could get current behavior with:

rc = partial(RetryingCaller(), httpx.HTTPError)

For all I care, I could add some syntactic sugar:

rc = RetryingCaller().on(httpx.HTTPError)

wdyt?

@ambv
Copy link
Contributor

ambv commented Jan 28, 2024

Are RetryingCaller instances supposed to be reused? To me it feels more natural to just call the damn function, without thinking about exceptions at the same time. IOW, the current design.

I like adding def on(...) on RetryingCaller, that lets you have both worlds, in case you want to customize an existing instance to respond to another exception. But actually calling an rc instance stays clean, without having to specify the exception every time.

@hynek
Copy link
Owner Author

hynek commented Jan 28, 2024

I think there's definitely the case for reuse where you want the same retry parameters but different errors to retry on.

@hynek
Copy link
Owner Author

hynek commented Jan 28, 2024

fixed/implemented in #57

hynek added a commit that referenced this pull request Jan 29, 2024
)

* Generalize retrying callers for sharing retry config w/o exc config

Ref #56 #45

* docs

* Use official name in public APIs

* Docs

* Fix docs build

* Stress

* words

* Add explanation

* Use exclude_also
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

3 participants