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

Update scanner.py #521

Closed
wants to merge 1 commit into from
Closed

Update scanner.py #521

wants to merge 1 commit into from

Conversation

blitzu
Copy link

@blitzu blitzu commented Jul 16, 2024

Here’s a modified version of the send_discovery_request function to include debugging and exception handling:

Here’s a modified version of the send_discovery_request function to include debugging and exception handling:
@jasonacox jasonacox requested a review from uzlonewolf July 17, 2024 03:16
iface['socket'].bind((address, 0))
except socket.gaierror as e:
print(f"Failed to bind to {address}: {e}")
continue
Copy link
Owner

Choose a reason for hiding this comment

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

Thanks for this PR @blitzu ! I'm wondering, why use print() instead of log.debug() and log.error() here?

Copy link
Author

Choose a reason for hiding this comment

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

this is what chatgpt gave me. i had an error on using scanner.py and this was the solution from chatgpt and it works

@uzlonewolf
Copy link
Collaborator

I'm gonna vote "no" for this one. PR #519 already fixes the underlying problem (instead of just hiding the error), print() should not be used here, and I like my spaces 🤣

@jasonacox
Copy link
Owner

and I like my spaces 🤣

Ha! I didn't even see them until @blitzu's PR. We could use a style guide helper. I'm tempted to cherry pick just the spaces fix for this PR. 😝

PR #519 already fixes the underlying problem (instead of just hiding the error)

That's true. I need to merge that and get into a minor rev release.

@blitzu even though this PR will be addressed with #519 , we appreciate you raising this! Active community members and willing contributors like you are how we keep the project moving forward. Thank you!

@uzlonewolf
Copy link
Collaborator

Personally I like having spaces inside brackets/parenthesis as I find it a lot more readable when scanning through code.

@uzlonewolf
Copy link
Collaborator

Full fix in #519.

@uzlonewolf uzlonewolf closed this Jul 25, 2024
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.

3 participants