Skip to content

Conversation

petrutlucian94
Copy link
Contributor

@petrutlucian94 petrutlucian94 commented Feb 1, 2019

py-amqp 2.4.0 caused a regression on Windows. Reading from a
non-blocking socket having the timeout set to 0 might raise a
WSAEWOULDBLOCK socket error instead of a timeout, which we're not
properly handling.

This change will make sure that we're just handling this as a
timeout, which will fix #252.

@codecov
Copy link

codecov bot commented Feb 1, 2019

Codecov Report

Merging #253 into master will decrease coverage by 0.16%.
The diff coverage is 25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #253      +/-   ##
==========================================
- Coverage   95.93%   95.77%   -0.17%     
==========================================
  Files          14       14              
  Lines        1748     1752       +4     
  Branches      256      258       +2     
==========================================
+ Hits         1677     1678       +1     
- Misses         44       47       +3     
  Partials       27       27
Impacted Files Coverage Δ
amqp/transport.py 87.5% <25%> (-0.89%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bf122a0...c75ffac. Read the comment docs.

@codecov
Copy link

codecov bot commented Feb 1, 2019

Codecov Report

Merging #253 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #253      +/-   ##
==========================================
+ Coverage   95.93%   95.94%   +<.01%     
==========================================
  Files          14       14              
  Lines        1748     1752       +4     
  Branches      256      258       +2     
==========================================
+ Hits         1677     1681       +4     
  Misses         44       44              
  Partials       27       27
Impacted Files Coverage Δ
amqp/transport.py 88.54% <100%> (+0.16%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bf122a0...96ace09. Read the comment docs.

auvipy
auvipy previously requested changes Feb 1, 2019
Copy link
Member

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

please fix the lint errors

@petrutlucian94
Copy link
Contributor Author

please fix the lint errors

Whops, sorry about that. Updated the commit.

@auvipy
Copy link
Member

auvipy commented Feb 1, 2019

thanks for fixing!! could you increase the test coverage a bit more?

py-amqp 2.4.0 caused a regression on Windows. Reading from a
non-blocking socket having the timeout set to 0 might raise a
WSAEWOULDBLOCK socket error instead of a timeout, which we're not
properly handling.

This change will make sure that we're just handling this as a
timeout.
@thedrow thedrow merged commit 2356f42 into celery:master Feb 4, 2019
@maingoh
Copy link

maingoh commented Feb 5, 2019

Tested and working ! Thanks a lot :)

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.

Publish raise socket.error: [Errno 10035] on Windows
3 participants