Skip to content

Comments

Support async actions#437

Merged
mergify[bot] merged 4 commits intojd:mainfrom
hasier:support-async-actions-2
Mar 2, 2024
Merged

Support async actions#437
mergify[bot] merged 4 commits intojd:mainfrom
hasier:support-async-actions-2

Conversation

@hasier
Copy link
Contributor

@hasier hasier commented Feb 6, 2024 •
edited
Loading

Fixes #249 #412

Second PR after breaking down #433 (follows #434)

After DRYing the iter() function, make AsyncRetrying support async callbacks.

Supersedes hasier#1

@hasier hasier mentioned this pull request Feb 6, 2024
Merged
@mergify
Copy link
Contributor

mergify bot commented Feb 6, 2024

⚠️ No release notes detected. Please make sure to use reno to add a changelog entry.

@hasier hasier mentioned this pull request Feb 6, 2024
Closed
jd
jd previously requested changes Feb 6, 2024
View reviewed changes
tenacity/_asyncio.py Outdated
WrappedFn = t.TypeVar("WrappedFn", bound=t.Callable[..., t.Awaitable[t.Any]])


def is_coroutine_callable(call: t.Callable[..., t.Any]) -> bool:
Copy link
Owner

@jd jd Feb 6, 2024

Choose a reason for hiding this comment

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

Could we have unit tests around this?

Copy link
Contributor Author

@hasier hasier Feb 6, 2024

Choose a reason for hiding this comment

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

Good call! I ended up moving the function to _utils and slightly amending it (odd partial behaviour in 3.8 and 3.9, even though docs say otherwise). wdyt? 2616c4a

@mergify mergify bot dismissed jd’s stale review February 6, 2024 12:46

Pull request has been modified.

@hasier hasier force-pushed the support-async-actions-2 branch from 323c008 to 2616c4a Compare February 6, 2024 12:52
@hasier hasier requested a review from jd February 6, 2024 12:55
jd
jd reviewed Feb 6, 2024
View reviewed changes
@hasier hasier mentioned this pull request Feb 12, 2024
Closed
@hasier
Copy link
Contributor Author

hasier commented Feb 13, 2024

@jd any more thoughts on this? Happy to clarify anything else I may have missed.

@hasier hasier requested a review from jd February 13, 2024 09:37
jd
jd previously approved these changes Feb 19, 2024
View reviewed changes
@mergify mergify bot dismissed jd’s stale review February 20, 2024 09:06

Pull request has been modified.

@hasier
Copy link
Contributor Author

hasier commented Feb 26, 2024

@jd I merged changes from main and tests seem to pass, feel free to re-approve and merge whenever you prefer 🙂

@jd jd added the no-changelog No changelog needed label Mar 2, 2024
@mergify mergify bot merged commit bfa2c80 into jd:main Mar 2, 2024
@hasier hasier deleted the support-async-actions-2 branch March 6, 2024 11:05
@hasier
Copy link
Contributor Author

hasier commented Mar 18, 2024 •
edited
Loading

@jd now that this is merged, do you think it's ready to be released in a new version? Or would you rather see some async strategies merged first too? The only issue I see with that is how we'd solve sync+async combinations 🤔 Gave a go at fixing the sync+async issue and it seems to be working, let me know what you think! #451

@hasier hasier mentioned this pull request Mar 18, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jd jd jd approved these changes

Assignees

No one assigned

Labels

no-changelog No changelog needed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Is an async retry_error_callback possible?

2 participants

@hasier @jd