DHCP graph for Vlan #2405
DHCP graph for Vlan #2405
Conversation
if there havent been added any new datapoints in gaphite recently (5 or 10 minutes it seems) then the previous function would not work, even if you changed start to 1year instead of 5 min
maybe shouldnt use max as a variable name
There was a problem hiding this comment.
This looks good so far, but the failing tests need to be addresses, as mentioned in my inline comments.
Also (and I realize this is on me, not having formally defined the metric path "standard" yet), the metric paths will not necessarilly conform to nav\.dhcp\.vlan\d+\.. As the original spec says in #2369:
The network names can also be something like vlan1100_some_description, or some_description_vlan1100, but this should still match as VLAN 1100 in NAV.
An extra level in the metric path for location may also be needed. This could in reality be any prefix configured into the integration script, something like:
VLAN numbers can be reused, even though they are separate broadcast domains. This typically happens on separate locations in a WAN, though - so be prepared that the path pattern may become a bit more complex...
| <h2>DHCP graphs</h2> | ||
|
|
||
| <ul class="small-block-grid-1 large-block-grid-2"> | ||
| {% if vlan.has_dhcp_stats %} |
There was a problem hiding this comment.
As evidenced by the failing tests, this introduces a dependency to a running Graphite server in the CI test suite - and the CI suite doesn't currently spin up a Graphite server. This causes the rendering of this page to crash with a GraphiteUnreachableError.
We could go about this by adding a Graphite server to the CI suite, but this page would still crash if the production Graphite becomes unreachable.
I suggest you handle this the same as some other places where the rendered HTML is dependent on querying Graphite: If Graphite isn't reachable, just display the error message "Graphite server is not reachable" instead of the graph widget (this is done where availability stats are concerned in the main tab of a device's ipdevinfo page).
There was a problem hiding this comment.
graphite error fixed, so i guess this is on hold until the metric path is defined
Codecov Report
@@ Coverage Diff @@
## master #2405 +/- ##
==========================================
+ Coverage 52.55% 52.62% +0.07%
==========================================
Files 552 552
Lines 40152 40165 +13
==========================================
+ Hits 21100 21136 +36
+ Misses 19052 19029 -23
Continue to review full report at Codecov.
|
this check was existed, then was removed in a previous commit when checking for connectiong errors to graphite, and now im bringing it back
|
Kudos, SonarCloud Quality Gate passed!
|
Fixes #2373
Graph dhcp statistics on vlan page