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

test/alternator: un-xfail a test which passes on modern Python #12038

Closed
wants to merge 1 commit into from

Conversation

nyh
Copy link
Contributor

@nyh nyh commented Nov 20, 2022

We had an xfailing test that reproduced a case where Alternator tried to report an error when the request was too long, but the boto library didn't see this error and threw a "Broken Pipe" error instead. It turns out that this wasn't a Scylla bug but rather a bug in urllib3, which overzealously reported a "Broken Pipe" instead of trying to read the server's response. It turns out this issue was already fixed in
urllib3/urllib3#1524

and now, on modern installations, the test that used to fail now passes and reports "XPASS".

So in this patch we remove the "xfail" tag, and skip the test if running an old version of urllib3.

Fixes #8195

@nyh nyh requested review from nuivall and havaker November 20, 2022 11:32
@scylladb-promoter
Copy link
Contributor

def test_too_large_request_chunked(dynamodb, test_table):
if Version(urllib3.__version__) < Version('1.26'):
pytest.skip("urllib3 before 1.26.0 threw broken pipe and did not read response and cause issue #8195. Fixed by pull request urllib3/rllib3#1524")
Copy link
Member

Choose a reason for hiding this comment

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

Typo in urllib3/rllib3#1524 (here and below copied):

Suggested change
pytest.skip("urllib3 before 1.26.0 threw broken pipe and did not read response and cause issue #8195. Fixed by pull request urllib3/rllib3#1524")
pytest.skip("urllib3 before 1.26.0 threw broken pipe and did not read response and cause issue #8195. Fixed by pull request urllib3/urllib3#1524")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Pushed a new version fixing this typo

We had an xfailing test that reproduced a case where Alternator tried
to report an error when the request was too long, but the boto library
didn't see this error and threw a "Broken Pipe" error instead. It turns
out that this wasn't a Scylla bug but rather a bug in urllib3, which
overzealously reported a "Broken Pipe" instead of trying to read the
server's response. It turns out this issue was already fixed in
   urllib3/urllib3#1524

and now, on modern installations, the test that used to fail now passes
and reports "XPASS".

So in this patch we remove the "xfail" tag, and skip the test if
running an old version of urllib3.

Fixes scylladb#8195
@scylladb-promoter
Copy link
Contributor

MeepoYang pushed a commit to storage-scsg/scylladb that referenced this pull request Apr 28, 2024
We had an xfailing test that reproduced a case where Alternator tried
to report an error when the request was too long, but the boto library
didn't see this error and threw a "Broken Pipe" error instead. It turns
out that this wasn't a Scylla bug but rather a bug in urllib3, which
overzealously reported a "Broken Pipe" instead of trying to read the
server's response. It turns out this issue was already fixed in
   urllib3/urllib3#1524

and now, on modern installations, the test that used to fail now passes
and reports "XPASS".

So in this patch we remove the "xfail" tag, and skip the test if
running an old version of urllib3.

Fixes scylladb#8195

Closes scylladb#12038
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.

Alternator's response to oversized requests confuses the client drivers
4 participants