Skip to content

Conversation

rohithasrk
Copy link
Contributor

@rohithasrk rohithasrk commented Aug 27, 2017

References #43. As of now, the basic parser code has been written. This code is to be tested properly with appropriate test cases. We've decided to use the package openvpn-status for the logic behind parsing openvpn logs.

To Dos:

  • Debug the code testing it with many test data
  • Develop an extensive test suite.
  • Think about adding the server IP.

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

You don't need to develop an extensive test suite, basic tests are good enough.

graph.add_edge(source, target, weight=1)
return graph

def _txtinfo_to_jsoninfo(self, data):
Copy link
Member

Choose a reason for hiding this comment

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

this is the method that should be called to_python (you can get rid of the other one).
You choose a starting point of another parser which was a special case.

Since you already have a library that parses the data, you don't really need to do much apart returning the data structure that the parser expects

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think you are right. We don't really need this method. Will edit


def _txtinfo_to_jsoninfo(self, data):
"""
converts openvpn txtinfo format to jsoninfo
Copy link
Member

Choose a reason for hiding this comment

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

jsoninfo is an olsr specific thing, you should not reuse this terminology

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh. I see. Will edit it

class OpenvpnParser(BaseParser):
""" OpenVPN status log parser """
protocol = 'openvpn'
version = ''
Copy link
Member

Choose a reason for hiding this comment

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

the status log file of openvpn has 2 versions, we could use this attribute to indicate which version we support

@coveralls
Copy link

coveralls commented Sep 8, 2017

Coverage Status

Coverage decreased (-7.3%) to 92.677% when pulling ad7d94e on rohithasrk:openvpn into 90e5eac on ninuxorg:master.

@rohithasrk
Copy link
Contributor Author

Oops! A wrong rebase.

@coveralls
Copy link

coveralls commented Sep 8, 2017

Coverage Status

Coverage decreased (-7.3%) to 92.677% when pulling 55ed0de on rohithasrk:openvpn into 90e5eac on ninuxorg:master.

@coveralls
Copy link

coveralls commented Sep 12, 2017

Coverage Status

Coverage decreased (-7.7%) to 92.255% when pulling bcda31e on rohithasrk:openvpn into 90e5eac on ninuxorg:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.8%) to 98.178% when pulling 908a7c3 on rohithasrk:openvpn into 90e5eac on ninuxorg:master.

@coveralls
Copy link

coveralls commented Sep 12, 2017

Coverage Status

Coverage decreased (-0.9%) to 99.089% when pulling c806817 on rohithasrk:openvpn into 90e5eac on ninuxorg:master.

@nemesifier
Copy link
Member

Thanks @rohithasrk, merged your commit in PR #47

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