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

fix AbstractTransport repr socket error (#361) #431

Merged
merged 1 commit into from
Jun 4, 2024
Merged

fix AbstractTransport repr socket error (#361) #431

merged 1 commit into from
Jun 4, 2024

Conversation

nburlett
Copy link
Contributor

If AbstractTransport's self.sock is disconnected, then self.sock.getpeername() will raise an error in repr, which shows up in backtraces for other upstream errors.

Fix this by catching socket.error in repr and putting the error info in to the returned repr string.

Fixes #361

@nburlett nburlett mentioned this pull request May 21, 2024
Closed
@Nusnus
Copy link
Member

Nusnus commented May 23, 2024 

  File "/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/docker/api/client.py", line 221, in _retrieve_server_version
    raise DockerException(
docker.errors.DockerException: Error while fetching server API version: Not supported URL scheme http+docker
  3.8-integration-rabbitmq: FAIL code 2 (8.98 seconds)
  evaluation failed :( (9.30 seconds)

Is due to docker/docker-py#3256

auvipy
auvipy reviewed May 23, 2024
View reviewed changes
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.

the changes seems OK. but lets wait until the CI is fully green again

auvipy
auvipy reviewed Jun 3, 2024
View reviewed changes
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.

can you please rebase it? should be passing after rebase

@nburlett
fix AbstractTransport repr socket error (celery#361)
7ae6c16 
If AbstractTransport's self.sock is disconnected, then
self.sock.getpeername() will raise an error in __repr__, which shows up
in backtraces for other upstream errors.

Fix this by catching socket.error in __repr__ and putting the error info
in to the returned repr string.

Fixes celery#361
@nburlett
Copy link
Contributor Author

nburlett commented Jun 3, 2024

can you please rebase it? should be passing after rebase

Done. tox is still failing locally for me, but the errors are the same for main and for fixes/361

Nusnus
Nusnus approved these changes Jun 3, 2024
View reviewed changes
Copy link
Member

@Nusnus Nusnus left a comment

Choose a reason for hiding this comment

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

LGTM

@Nusnus
Copy link
Member

Nusnus commented Jun 3, 2024

@auvipy - CI is passing, giving you the last word here

@auvipy
Copy link
Member

auvipy commented Jun 4, 2024

thanks all for your efforts and insights!

@auvipy auvipy merged commit db7af5f into celery:main Jun 4, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@auvipy auvipy auvipy approved these changes

@Nusnus Nusnus Nusnus 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.

Potentially broken _AbstractTransport.__repr__
3 participants
@nburlett @Nusnus @auvipy