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

Allow message to match on multiple OS #245

Merged

Conversation

luke-orden
Copy link
Collaborator

This fixes #242

Currently when a matching OS is found we do not check remaining OS. This
mean that if 2 OS have message prefixes that match each other's logs
then you cannot be sure that the message will be forwarded onto the
correct device process.

I have updated so the message is now forwarded onto all matching os, not
just the first match.

To allow the tests to work correctly I have removed 'send_raw': True
from the config test.

@todo
Copy link

todo bot commented May 29, 2018

Should we stop searching and just return, or should we return all matches OS?

# TODO Should we stop searching and just return, or should we return all matches OS?
return msg_dict
def _identify_os(self, msg):
'''
Using the prefix of the syslog message,


This comment was generated by todo based on a TODO comment in a839c02 in #245. cc @loverend.

@coveralls
Copy link

coveralls commented May 29, 2018

Coverage Status

Coverage increased (+0.2%) to 43.173% when pulling 15adfe3 on loverend:send_to_multiple_os into fa78472 on napalm-automation:develop.

This fixes napalm-automation#242

Currently when a matching OS is found we do not check remaining OS. This
mean that if 2 OS have message prefixes that match each other's logs
then you cannot be sure that the message will be forwarded onto the
correct `device` process.

I have updated so the message is now forwarded onto all matching os, not
just the first match.

To allow the tests to work correctly I have removed `'send_raw': True`
from the config test.
@luke-orden
Copy link
Collaborator Author

Removed the todo mentioned above as it is no longer relevant.

@mirceaulinic mirceaulinic self-requested a review May 29, 2018 14:14
@mirceaulinic mirceaulinic added this to the 0.5.1 milestone May 29, 2018
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.

Can you please fix also the py3 issues so the tests run cleanly:

=================================== FAILURES ===================================
_______________________________________  _______________________________________
napalm_logs/listener/kafka.py:17:1: W0612 local variable 'err' is assigned to but never used [pyflakes]
_______________________________________  _______________________________________
napalm_logs/listener/zeromq.py:16:1: W0612 local variable 'err' is assigned to but never used [pyflakes]
_______________________________________  _______________________________________
napalm_logs/transport/kafka.py:15:1: W0612 local variable 'err' is assigned to but never used [pyflakes]

This fixes the following:

```
=================================== FAILURES
===================================
_______________________________________
_______________________________________
napalm_logs/listener/kafka.py:17:1: W0612 local variable 'err' is assigned to but never used [pyflakes]
_______________________________________
_______________________________________
napalm_logs/listener/zeromq.py:16:1: W0612 local variable 'err' is assigned to but never used [pyflakes]
_______________________________________
_______________________________________
napalm_logs/transport/kafka.py:15:1: W0612 local variable 'err' is assigned to but never used [pyflakes]
```
@mirceaulinic mirceaulinic merged commit ebc36d2 into napalm-automation:develop May 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Init for different OS match but only process one
3 participants