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

Tests: retry request on ConnectionError. #654

Merged
merged 1 commit into from
Dec 31, 2018

Conversation

mauritsvanrees
Copy link
Member

On Jenkins we often get one ConnectionError in a seemingly random test.
Happens on both Python 2 and 3.
Retrying after a short pause helps.
Fixes #648

Note: I added a print statement so we know if it happens, and maybe detect a pattern in the urls where this goes wrong. I tried logger.warning and logger.error, but this never showed up in the test output.

In a few local parallel runs, I have seen errors on these urls:

http://localhost:64140/plone/@users
http://localhost:64117/plone/@types/Document
http://localhost:49516/plone/@users/noam

@mauritsvanrees
Copy link
Member Author

Meh, bin/code-analysis on Travis does not like the print statement. Fair enough. I will remove it.

On Jenkins we often get one ConnectionError in a seemingly random test.
Happens on both Python 2 and 3.
Retrying after a short pause helps.
Fixes #648
@mauritsvanrees mauritsvanrees force-pushed the issue-648-retry-on-connectionerror branch from e69705e to e6b601c Compare December 31, 2018 12:08
@mauritsvanrees
Copy link
Member Author

I have amended the commit. This was the print statement if anyone wants to try that:

print('\n### Got ConnectionError at %s, retrying once.' % url)

To get the url out from amidst the many deprecation and other warnings:

bin/test -s plone.restapi -m test_documentation 2>&1 | grep ConnectionError

@Rotonen
Copy link
Contributor

Rotonen commented Dec 31, 2018

Is there something which still needs to happen after the ZServer ProcessStarting event fires before plone.rest or plone.restapi endpoints are actually available? Or is that somehow different for Python 3 / WSGI?

The root cause should probably get figured out eventually.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 96.225% when pulling e6b601c on issue-648-retry-on-connectionerror into 8f4b1e3 on master.

@mauritsvanrees
Copy link
Member Author

Agreed, the root cause would be good to know.
But for now, the Jenkins jobs are green, except for one with a spurious error in plone.i18n in Japanese. So should be safe to merge.

Copy link
Member

@gforcada gforcada left a comment

Choose a reason for hiding this comment

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

LGTM, maybe rather than a print statement, we can add one deprecation warning, it is not perfect, but does its job, I.e. get printed on Jenkins log output 😃

@gforcada gforcada merged commit e8a9f4f into master Dec 31, 2018
@gforcada gforcada deleted the issue-648-retry-on-connectionerror branch December 31, 2018 18:03
@gforcada
Copy link
Member

And we, hopefully, will finish the year with Jenkins green 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants