-
Notifications
You must be signed in to change notification settings - Fork 3
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
Added Internal Metadata table #30
Conversation
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.
Looks good to me! Just left two comments
|-----------------------|-------------|------------|-------|-----------------------------------------------|------------|-------------------------------| | ||
| liveness_status | topology | link | str | Link's liveness status up\|down | 06.30.22 | topology, core | | ||
| last_status_change | topology | link | float | Link's last status change timestamp | 06.30.22 | topology | | ||
| last_status_is_active | topology | link | bool | Whether Link's last status is active or not | 06.30.22 | topology | |
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 wonder if it wouldn't be simple if instead of last_status_is_active
we just store last_status
(which could also store other values in the future (in maintenance, maybe?) - not sure if it is actually useful).
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.
@italovalcy great idea, I've mapped this enhancement for the next release kytos-ng/topology#99. Also, there's another similar opportunity with last_status_change
, currently it stores a float epoch, which works great. But, if it were stored as datetime, especially since it's a metadata value, it would provide a better UX for providers who could be able to read it without conversions, and also on MongoDB
its type would be ISODate
which would enable queries based on datetime constraints without conversions, I haven't mapped this as an enhancement though, other than documenting on that comment that I linked here. Let me know what you think and if we should map this enhancement too. Appreciated your review and inputs.
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've mapped it here kytos-ng/topology#104
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.
@italovalcy thanks for reviewing and the suggestions, once these new two issues are addressed we can update this page again.
Co-authored-by: Italo Valcy S Brito <[email protected]>
We've been using also internal metadata that has different semantics from general metadata, so I've added a table docummenting which internal metadata keys our NApps have been using and their purpose. I've also added a paragraph explaining the difference.
We don't have validations on endpoints protecting internal metadata yet, but that will be covered on this issue kytos-ng/topology#51 in the future.