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

Feature/nxos ssh get interfaces counters #1287

Conversation

ExaneServerTeam
Copy link
Contributor

=Implements nxos_ssh get_interfaces_counters=

  • Performs the parsing thought the json pipped output.
  • mgmt specific case is taken into account
  • test included

@coveralls
Copy link

coveralls commented Sep 2, 2020

Coverage Status

Coverage remained the same at 0.0% when pulling a4034a7 on ExaneServerTeam:feature/nxos_ssh_get_interfaces_counters into 4b17b93 on napalm-automation:develop.

@ExaneServerTeam ExaneServerTeam force-pushed the feature/nxos_ssh_get_interfaces_counters branch from 54c2701 to fa5b17c Compare September 2, 2020 06:59
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.

Looks good, just please make sure the tests are passing, then we should be gtg.


return all_stats_d

# print(counters_table_raw)
Copy link
Member

Choose a reason for hiding this comment

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

Please remove

'rx_broadcast_packets': int,
"""
if_mapping = {
'eth': {
Copy link
Member

Choose a reason for hiding this comment

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

I think Black doesn't like this, can you upgrade to the latest version then run black .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mirceaulinic Thanks for the review.
I fix it asap.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mirceaulinic Fixes on their way with latest push.

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.

LGTM. Thanks @ExaneServerTeam!

@mirceaulinic mirceaulinic merged commit 0028952 into napalm-automation:develop Sep 2, 2020
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.

None yet

3 participants