Uninett / zino

Zino 2.0 - Network state monitor for research networks
Apache License 2.0
3 stars 4 forks source link

Replace overloaded `port` concept for indexing of events #161

Closed lunkwill42 closed 5 months ago

lunkwill42 commented 5 months ago

The original Zino 1 Tcl code used port as one of three components used to uniquely index events. However, it greatly overloaded the meaning of port.

For reachability events, port would be an empty value, since the event would pertain to the router itself, and not to a sub-component of the router. For actual port state events, it would be the ifindex value of the port in question. For BGP events, it would be the IP address of the BGP peer, and so forth.

We took this concept and ran with it in the initial stages of porting the code to Python, but the concept leaked into the attributes of the Event base class, causing all event types to have a port attribute, which made no sense. This became rather apparent when writing a converter for Zino 1 state to Zino 2 state.

So, this is the time to get rid of this concept and be more explicit about an index value being an index value, and not a port name or identifier.

Each event type, with the notable exception of reachability events, has some attribute that uniquely identifies it among events of the same type for the same router. This attribute is named differently for the different event types. Since some of the existing event types relied on its inherited port attribute for this, this PR adds new attributes to these event types (to match the names given in the Zino 1 API), and replaces the concept of port for indexing with a generic subindex term.

The pre-existing PortOrIPAddress compound type annotation was mean to represent these index values, but was badly named (as it encompassed more than just ports and IP addresses). This was renamed and repurposed into a SubIndex compound type, which can now be extended if new event types are ever added.

The PR also fixes a few small bugs and issues with how event attributes were set (or not set) by the various implemented tasks.

github-actions[bot] commented 5 months ago

Test results

    3 files      3 suites   26s :stopwatch: 224 tests 224 :heavy_check_mark: 0 :zzz: 0 :x: 672 runs  670 :heavy_check_mark: 2 :zzz: 0 :x:

Results for commit 4885340d.

:recycle: This comment has been updated with latest results.

hmpf commented 5 months ago

cannot resist: I did tell you so :)

lunkwill42 commented 5 months ago

The only small request that I have is that the docstrings for get_or_create_event and create_event should be adjusted to indicate that event though a subindex is given it will not automatically be set for the field that represents the subindex.

Pushed one more commit to this effect, then.

codecov[bot] commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (074c385) 0.00% compared to head (ec8711d) 0.00%.

:exclamation: Current head ec8711d differs from pull request most recent head 4885340. Consider uploading reports for the commit 4885340 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #161 +/- ## ============================= ============================= ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

sonarcloud[bot] commented 5 months ago

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud