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

[juniper] Removing last trunk member errors out #37

Merged

Conversation

mat128
Copy link
Contributor

@mat128 mat128 commented Feb 25, 2016

Removing the last trunk member of a trunk interface results
in an error at commit time.

@godp1301
Copy link
Contributor

👍

@@ -495,3 +500,9 @@ def get_or_create_interface(if_list, port):

return existing


def port_is_in_access_mode(port):
return port.mode is None or port.mode == "access"
Copy link
Contributor

Choose a reason for hiding this comment

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

What are the conditions to have port.mode == None?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lindycoder
Copy link
Contributor

👍

@JoProvost
Copy link
Contributor

👎 tests are failing

@mat128
Copy link
Contributor Author

mat128 commented Feb 25, 2016

They all passed on my machine :( Looks like the order is important in the XML (yeah right)... Trying to reproduce locally.

@mat128 mat128 force-pushed the juniper_cannot_remove_last_trunk_member branch 5 times, most recently from c0aaab1 to cc2402a Compare March 1, 2016 18:24
@@ -335,19 +335,19 @@ def apply_trunk_native_vlan(self, interface_data, port):
def get_trunk_native_vlan_node(self, interface_node):
return interface_node.xpath("unit/family/ethernet-switching/native-vlan-id")

@staticmethod
Copy link
Contributor

Choose a reason for hiding this comment

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

reason?

Copy link
Contributor

Choose a reason for hiding this comment

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

i mean why static?
I get the "it doesn't use self" argument, but philsophilcally, it does not act on the class, of the instances but rather a a simple extraction of an instance method, i don't think that qualifies it to be static. and maybe it's also worth being _underscored as a protected method

@emmurd
Copy link
Contributor

emmurd commented Mar 1, 2016

LGTM 👍

@lindycoder
Copy link
Contributor

👎 just in case i wasn't clear

@mat128 mat128 force-pushed the juniper_cannot_remove_last_trunk_member branch from cc2402a to 1af706d Compare March 2, 2016 05:28
@mat128
Copy link
Contributor Author

mat128 commented Mar 2, 2016

Alright, addressed latest comments in last push.

@mat128 mat128 force-pushed the juniper_cannot_remove_last_trunk_member branch from 1af706d to eeaff03 Compare March 2, 2016 13:13
@lindycoder
Copy link
Contributor

👍

emmurd added a commit that referenced this pull request Mar 2, 2016
…ember

[juniper] Removing last trunk member errors out
@emmurd emmurd merged commit dad0360 into internap:master Mar 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants