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.

Sign up for GitHub

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

Jump to bottom

Updating an interface via the api and sending lowercase mac_address creates a pointless changelog #17797

Open
ITJamie opened this issue Oct 17, 2024 · 6 comments · May be fixed by #18430
Open

Updating an interface via the api and sending lowercase mac_address creates a pointless changelog #17797

ITJamie opened this issue Oct 17, 2024 · 6 comments · May be fixed by #18430
Assignees
@atownson
Labels
netbox severity: low Does not significantly disrupt application functionality, or a workaround is available status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application

Comments

@ITJamie
Copy link
Contributor

ITJamie commented Oct 17, 2024

Deployment Type

Self-hosted

Triage priority

N/A

NetBox Version

v4.1.4

Python Version

3.12

Steps to Reproduce

create an interface on a device. set a mac address.
via the api update the interface with the same mac address but send it in lowercase form instead of uppercase form

Expected Behavior

if the mac addresses match (ignoring case), no changelog should be made

Observed Behavior

a change log is created showing the mac_address being set to lowercase. but its stored in the db in uppercase. so there was no need for the changelog being created
@ITJamie ITJamie added status: needs triage This issue is awaiting triage by a maintainer type: bug A confirmed report of unexpected behavior in the application labels Oct 17, 2024
@ITJamie
Copy link
Contributor Author

ITJamie commented Oct 17, 2024

Screenshot 2024-10-16 at 20 22 04 Example of the changelog thats created. but viewing the interface directly after this changelog shows that its stored as uppercase

@arthanson arthanson added status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation severity: low Does not significantly disrupt application functionality, or a workaround is available and removed status: needs triage This issue is awaiting triage by a maintainer labels Oct 17, 2024
@betheapi
Copy link

I would like to work on this issue

@jeremystretch
Copy link
Member

Thanks @betheapi, I've assigned this to you. For reference, the MAC address should always be normalized to lowercase.

@jeremystretch jeremystretch added the netbox label Nov 1, 2024 — with Linear
@arthanson arthanson added status: accepted This issue has been accepted for implementation and removed status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation labels Dec 3, 2024
@jeremystretch jeremystretch added status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation and removed status: accepted This issue has been accepted for implementation labels Dec 26, 2024
@atownson
Copy link
Contributor

I can work on this one.

@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation and removed status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation labels Jan 17, 2025
@atownson
Copy link
Contributor

@jeremystretch,
Based on the comment above, MAC Addresses should be normalized to lowercase, correct?
This line normalizes to uppercase:

netbox/netbox/dcim/fields.py

Line 43 in f845b2c

return EUI(value, version=48, dialect=mac_unix_expanded_uppercase)

I can change this, but wanted to confirm the desired normalization.

@jeremystretch
Copy link
Member

I might be wrong then, sorry. Do whatever is most consistent with existing behavior.

@atownson atownson linked a pull request Jan 17, 2025 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@atownson atownson

Labels
netbox severity: low Does not significantly disrupt application functionality, or a workaround is available status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixes #17797 - Update MACAddressSerializer to use MACAddressField atownson/netbox
5 participants
@arthanson @ITJamie @jeremystretch @atownson @betheapi