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

Use deferred to be full async in retry #101

Merged
merged 3 commits into from
Jun 22, 2018
Merged

Use deferred to be full async in retry #101

merged 3 commits into from
Jun 22, 2018

Conversation

joelwurtz
Copy link
Member

@joelwurtz joelwurtz commented Apr 5, 2018

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
License MIT

What's in this PR?

It adds a deferred class which allow to resolve retry in an async way (no more wait call)

Why?

Previously using retry plugin would force async to be sync

Checklist

  • Need to updated CHANGELOG.md (wait for other PR to be merged to not have useless conflicts)

@dbu
Copy link
Contributor

dbu commented May 28, 2018

this can now be rebased and made ready for merging ;-)

@joelwurtz joelwurtz force-pushed the feature/deferred-retry branch from aa724f0 to 353e66c Compare May 28, 2018 14:11
@joelwurtz
Copy link
Member Author

It should be good now.

However this plugin may still need a warning, as we have a call to the sleep function inside which is synchronous, there is the possibility to have asynchronous sleep but it heavily depends on the implementation (amp / react / ...), so there is no possibility to add it here

@dbu
Copy link
Contributor

dbu commented May 28, 2018

However this plugin may still need a warning, as we have a call to the sleep function inside which is synchronous

do you want to add something for this topic to the documentation of the retry plugin?

@joelwurtz
Copy link
Member Author

joelwurtz commented Jun 22, 2018

Done in php-http/documentation#237

@dbu dbu merged commit 949ee8a into master Jun 22, 2018
@joelwurtz joelwurtz deleted the feature/deferred-retry branch June 22, 2018 14:23
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.

2 participants