Skip to content

[385] Fix for stuck consumers due to unsafe host key access #386

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

Conversation

moogzy
Copy link
Contributor

@moogzy moogzy commented May 12, 2025

This PR addresses: #385

Vendors like IOS-XR by default send messages with no hostname. The napalm-logs ios-xr parser was updated to support this default behaviour. However, server.py has unsafe access logic for the host key in msg_dict which can cause the consumer/listeners to become stuck as they are expecting messages to have a host key for lookup.

This PR turns makes the host key access safe and returns a default value of "unknown" if the host key doesn't exist.

Signed-off-by: Adrian Arumugam [email protected]

@moogzy moogzy marked this pull request as ready for review May 12, 2025 15:16
@moogzy
Copy link
Contributor Author

moogzy commented May 12, 2025

@mirceaulinic if you have a moment to review please :)

Vendors like IOS-XR by default send messages with no hostname. Log parsers were updated to support this.

This PR fixes the server.py unsafe access for the host key in msg_dict which cause the consumer/listeners to become stuck
as they are expecting messages to have a host key for lookup.

Signed-off-by: Adrian Arumugam <[email protected]>
@moogzy moogzy force-pushed the moogzy/fix-unsafe-host-key-access branch from dbb723e to 1ff10fa Compare May 16, 2025 13:40
Copy link
Member

@mirceaulinic mirceaulinic left a comment

Choose a reason for hiding this comment

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

Apologies for getting back so late to you @moogzy - I've been on a sabathical for a few months.

Thank you for your submission! The changes make sense

@mirceaulinic mirceaulinic merged commit cc19a10 into napalm-automation:develop Jul 2, 2025
@moogzy
Copy link
Contributor Author

moogzy commented Jul 2, 2025

No dramas @mirceaulinic

I hope you had a nice break! Welcome back 😁

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.

2 participants