-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Adding asdot notation to ASN views #8329
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
Adding asdot notation to ASN views #8329
Conversation
Adds custom property to asn model to compute asdot notation if required. Updates asn view to show asdot notation if one exists in the format xxxxx (yyy.yyy) Adds a custom column renderer to asn table to display asdot notation if one exists
|
@jeremystretch Soliciting some feedback on this. Should the page title also be updated to include the asdot notation or not? |
|
Sure? No strong opinion here. |
jeremystretch
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.
Thanks for this! Just had a couple suggestions.
netbox/ipam/models/ip.py
Outdated
| def __str__(self): | ||
| return f'AS{self.asn}' | ||
| if self.asn > 65535: | ||
| return 'AS{} ({}.{})'.format(self.asn, self.asn // 65536, self.asn % 65536) |
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.
This should probably return self.asdot_notation.
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.
I think we should also stick with fstrings if we can IMO
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.
Updated to use fstring and to return asn_with_asdot per below suggestion. Resulting code is much cleaner.
netbox/ipam/models/ip.py
Outdated
| return reverse('ipam:asn', args=[self.pk]) | ||
|
|
||
| @property | ||
| def asdot_notation(self): |
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.
Maybe rename this to asn_with_asdot or something like that, and return either the ASN with ASDOT notiation or just the ASN. Then you can reference it directly from within the ASNTable and object template.
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.
Great suggestion. That works much cleaner. Updated ASNTable & object template.
* Updating asdot computation to use an fstring * Cleaning code. Custom property now returns either the ASN with ASDOT notation or just the ASN. asn_with_asdot can now be referenced in ASNTable & objet template.
|
Excellent, thanks @jasonyates! |

Fixes: #8293
Adds custom property to asn model to compute asdot notation if required.
Updates asn view to show asdot notation if one exists in the format xxxxx (yyy.yyy)
Adds a custom column renderer to asn table to display asdot notation if one exists