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.

Sign up for GitHub

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

Jump to bottom

feat: Add delayed retry on 500/503 responses. #122

Merged
merged 7 commits into from
Jul 30, 2019
Merged

feat: Add delayed retry on 500/503 responses. #122

merged 7 commits into from
Jul 30, 2019

Conversation

jrconlin
Copy link
Member

Closes #121

@jrconlin jrconlin requested review from pjenvey and fzzzy June 24, 2019 18:25
@jrconlin
Copy link
Member Author

Since get doesn't impose the lock requirement, it's not really required to go through the retry method wrapper.

@pjenvey pjenvey requested a review from rfk June 24, 2019 19:32
rfk
rfk reviewed Jun 25, 2019
View reviewed changes
Copy link
Contributor

@rfk rfk left a comment

Choose a reason for hiding this comment

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

👍 to doing this work in the tests rather than in the app. It may be worth getting rid of the sleep_and_retry_on_conflict decorator entirely if this helps the tests do that some work more explicitly.

syncstorage/tests/functional/test_storage.py Outdated Show resolved Hide resolved
syncstorage/tests/functional/test_storage.py Outdated Show resolved Hide resolved
@jrconlin jrconlin added the WIP label Jun 26, 2019
@jrconlin jrconlin force-pushed the feat/retry branch 2 times, most recently from 1b1014a to 494c8ac Compare June 26, 2019 21:05
@jrconlin jrconlin removed the WIP label Jun 27, 2019
@pjenvey pjenvey mentioned this pull request Jul 3, 2019
Closed
@jrconlin
f Use test prefixed 'xxx*' container names
496fce2 
For stand-alone, memcache based test, probably not a concern, but if
these tests are run against a shared database (say one being used for
rust tests), the tests will create a number of containers. We should
note these so that they can be easily cleaned up later, as well as
possibly avoid potential issues with existing, future collections.

Test collections are now prefixed with "xxx" (e.g. "xxxx", "xxx_col",
"xxx_meh", etc.)
fzzzy
fzzzy approved these changes Jul 30, 2019
View reviewed changes
Copy link
Contributor

@fzzzy fzzzy left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This branch makes the functional failures I see when running against syncstorage-rs drop from between 18-20 (intermittent) to 1 (known failure that will be fixed by the actix-1.0 upgrade)

@jrconlin jrconlin merged commit d92ef07 into master Jul 30, 2019
@pjenvey pjenvey deleted the feat/retry branch July 31, 2019 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rfk rfk rfk left review comments

@pjenvey pjenvey pjenvey approved these changes

@fzzzy fzzzy fzzzy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add delayed retry on 500/503 responses
4 participants
@jrconlin @pjenvey @rfk @fzzzy