-
Notifications
You must be signed in to change notification settings - Fork 260
Allow dict parameter query for primary_mac_address in netbox_vm_interface #1510
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 dict parameter query for primary_mac_address in netbox_vm_interface #1510
Conversation
|
Great, I'll take a look when I get a chance |
|
I hate doing this but I think the additional cleanups you did need to be pulled out of this and a separate PR since they are not part of the main subject of the PR. Unfortunately because they are combined in the same commit as opposed to being a separate commit I am not able to filter it out. Thank you for doing them, if we could just cherry-pick that commit into a separate branch and a PR, so that I can focus on just the main issue in this PR that would be great. Thanks |
Looks like lint is now failing on files I haven't touched. Do you want me to fix those? |
sc68cal
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small nitpick, but otherwise I think this is good to go after small typo correction. Thank you for your contribution
| @@ -0,0 +1,3 @@ | |||
| --- | |||
| bugfixes: | |||
| - "Fix specifying primary_mac_address objectsby id on vm interfaces for disambiguation" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - "Fix specifying primary_mac_address objectsby id on vm interfaces for disambiguation" | |
| - "Fix specifying primary_mac_address objects by id on vm interfaces for disambiguation" |
If you want to, but I would prefer it in a separate PR. |
|
Actually I just went ahead and did it. You should be able to rebase your PR once I merge it. |
…face * Allow dict parameter query for primary_mac_address in netbox_vm_interface (str->raw) * Add integration tests for adding primary_mac_address to vminterface by id * Add changelog fragment * A few syntax, spelling, ordering fixes I can remove if requested
b23c622 to
170e4d6
Compare
Allow dict parameter query for primary_mac_address in netbox_vm_interface module
Related Issue
#1509
New Behavior
This PR adds the ability to query for a primary_mac_address by ID for vm interfaces. Example of new behavior:
This PR also addresses a potential bug. The primary_mac_address was not specified under QUERY_TYPES in netbox_utils.py and as a result querys by ID would also result in matching mac addresses whose value contains the ID number (so if the MAC is 00:01:02:03:42:43 and the ID of another MAC address object is 42, then the query would return both a fail.
Contrast to Current Behavior
Currently, a primary_mac_address may only be assigned by its value.
Discussion: Benefits and Drawbacks
This is also related to #1413 as-well-as the PR submitted to fix that issue #1464 where duplicate mac addresses meant assigning a primary_mac_address to device interfaces was not always possible. This PR adds that same functionality to vm interfaces. The original behavior of specifying a primary_mac_address by value is still supported.
Proposed Release Note Entry
Allow dict parameter query for primary_mac_address in netbox_vm_interface module.
Double Check
develbranch.