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

Add PortAdmin "Write mem" support for H3C switches #1515

Merged
merged 7 commits into from Jun 6, 2017

Conversation

jmbredal
Copy link
Collaborator

@jmbredal jmbredal commented May 9, 2017

Related issue #1411

@jmbredal jmbredal requested a review from lunkwill42 May 9, 2017 12:13
@lunkwill42 lunkwill42 self-assigned this May 31, 2017
@lunkwill42 lunkwill42 changed the title H3c write mem Add PortAdmin "Write mem" support for H3C switches May 31, 2017
@lunkwill42 lunkwill42 added this to the 4.7.0 milestone May 31, 2017
Copy link
Member

@lunkwill42 lunkwill42 left a comment

Choose a reason for hiding this comment

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

I've done a very cursory write-test from PortAdmin (not on an H3C, though). Have not tested Arnold. Since both subsystems using SNMP SET operations are in your hands, I guess you are best qualified to assess any effects on these. I will assume read functionality is unaffected by this.

My only comment is therefore about the slight API change, noted inline.

(TYPEMAP.keys(), data_type))
return value_type

def set(self, query, data_type, value):
Copy link
Member

Choose a reason for hiding this comment

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

Although a variable named type is generally a no-no, this is part of the API, and making the API inconsistent is a bigger no-no.

Either keep type, or rename it to data_type in the set method of all the other class Snmp implementations - and ensure that any calling code that is using keyword arguments is updated.

@lunkwill42 lunkwill42 merged commit 2b43657 into Uninett:master Jun 6, 2017
@jmbredal jmbredal deleted the h3c_write_mem branch October 23, 2017 07:37
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

2 participants