-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add planned maintenance #194
Conversation
Test results 3 files 3 suites 51s ⏱️ Results for commit db797f8. ♻️ This comment has been updated with latest results. |
e617895
to
ca0a568
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #194 +/- ##
==========================================
+ Coverage 98.98% 98.99% +0.01%
==========================================
Files 50 50
Lines 6565 6740 +175
==========================================
+ Hits 6498 6672 +174
- Misses 67 68 +1 ☔ View full report in Codecov by Sentry. |
I just checked in Zino1.0 and found that |
yeah noticed this too, seems like an oversight but I guess no planned maintenance has ever been shorter than whatever interval the PM stuff runs at |
Yes, that would make very little sense to have a PM shorter than one minute. |
bb41ab8
to
c7ada43
Compare
Quality Gate passedIssues Measures |
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 don't know the original PM code intimately, so I'm just going by intuition on some of the functionality.
All in all, pretty comprehensive and nearly complete, but I'm left with a feeling that perhaps more of the functionality of the PlannedMaintenances
class could be moved to the PlannedMaintenance
class, especially much of the things that just take a single PlannedMaintenance
object and doesn't really touch the internal state of the PlannedMaintenances
object itself.
Another idea could be to split PlannedMaintance
class could even be split into subclasses, one for each type ("portstate", "device"). The event matching or generating methods could then be subclass specific.
Maybe an IRL pow-wow would be better here :)
3d465f7
to
b980219
Compare
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 is really turning out nice!
Cannot finish without some nit-picks, though! :-)
🦙 MegaLinter status: ✅ SUCCESS
See detailed report in MegaLinter reports |
This is very similar to the Events model, just missing the index
This task checks regularly if any planned maintenances have started/stopped and handles affected events
Co-authored-by: Morten Brekkevold <[email protected]>
This fixes obvious renames and typos from code review. Co-authored-by: Johanna England <[email protected]>
The original event object is not modified externally. We need to fetch the updated copy from the event registry. Co-authored-by: Johanna England <[email protected]>
c9df563
to
db797f8
Compare
Quality Gate passedIssues Measures |
Fixes #61, API still missing (tracked by #101). That will be separate PR