Add VLAN netident to ipdevinfo ports table#3473
Conversation
Test results 27 files 27 suites 45m 17s ⏱️ Results for commit b23fc35. ♻️ This comment has been updated with latest results. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3473 +/- ##
=======================================
Coverage 62.67% 62.67%
=======================================
Files 611 611
Lines 45100 45107 +7
Branches 43 43
=======================================
+ Hits 28266 28272 +6
- Misses 16824 16825 +1
Partials 10 10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
fe9a66b to
9f6644a
Compare
9f6644a to
ab9ce74
Compare
|
Good call. Never post customer data online unless you have explicit consent. |
There was a problem hiding this comment.
The issue mentions #1935, which was solved by #1955 - it basically uses str(vlan_object) as "description" - which, in fact, is the netident, not the "description". Netidents are more prevalent.
But it sounds like you need input on what SwportVlan is and how to interpret the data, so let's have a f2f chat about this feature next week...
lunkwill42
left a comment
There was a problem hiding this comment.
Works very well for me, only nitpicks left :)
| """ | ||
|
|
||
| queryset = manage.Interface.objects.all() | ||
| queryset = manage.Interface.objects.prefetch_related('swport_vlans__vlan').all() |
There was a problem hiding this comment.
This isn't strictly necessary for the new feature to work, however it is here for performance reasons - which means it should be in a separate commit that explains why it was added :)
| @property | ||
| def vlan_description(self): | ||
| if self.trunk: | ||
| return None | ||
| swportvlan = self.swport_vlans.first() | ||
| return swportvlan.vlan.net_ident if swportvlan else None |
There was a problem hiding this comment.
Strictly speaking, this should be called vlan_netident. Other than that, it seems you forgot the discussed docstring to explain that this doesn't return results for trunk ports, since they carry multiple VLANs.
e62a707 to
dc406b7
Compare
dc406b7 to
b23fc35
Compare
|
Scope and purpose
Fixes #2160.
Adds new column showing VLAN netident to the Ports table in ipdevinfo.
Adds VLAN netident to the results of the
interfaceAPI. This was necessary as the Port table gets its data from that endpoint.Uses
prefetch_relatedto reduce database lookups. We access the correctSwPortVlanviaself.swport_vlans, which returns a set, soselect_relatedcannot be used.This was originally called "Vlan Description", but was changed to netident as the NAV Vlan class has both a description and netident value, but the netident value is the most relevant to use here
This pull request
Contributor Checklist
Every pull request should have this checklist filled out, no matter how small it is.
More information about contributing to NAV can be found in the
Hacker's guide to NAV.
<major>.<minor>.x). For a new feature or other additions, it should be based onmaster.