Skip to content

Allow expiration date for downtime#426

Merged
jose-caballero merged 2 commits intomainfrom
allow_expiration_date_for_downtime
Jan 26, 2026
Merged

Allow expiration date for downtime#426
jose-caballero merged 2 commits intomainfrom
allow_expiration_date_for_downtime

Conversation

@jose-caballero
Copy link
Contributor

@jose-caballero jose-caballero commented Jan 13, 2026

Description:

Special Notes:


Submitter:

Have you (where applicable):

  • Added unit tests?
  • Checked the latest commit runs on Dev?
  • Updated the example config file(s) and README?

Reviewer

Does this PR:

  • Place non-StackStorm code into the lib directory?
  • Have unit tests for the action/sensor and lib layers?
  • Have clear and obvious action parameter names and descriptions?

@codecov
Copy link

codecov bot commented Jan 13, 2026

Codecov Report

❌ Patch coverage is 95.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 99.81%. Comparing base (e4d97e6) to head (dee169d).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
lib/workflows/hypervisor_downtime.py 95.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #426      +/-   ##
==========================================
- Coverage   99.88%   99.81%   -0.08%     
==========================================
  Files         109      109              
  Lines        2598     2638      +40     
  Branches      320      329       +9     
==========================================
+ Hits         2595     2633      +38     
- Misses          3        4       +1     
- Partials        0        1       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jose-caballero jose-caballero force-pushed the allow_expiration_date_for_downtime branch 2 times, most recently from 3c21d0a to 037091c Compare January 14, 2026 07:47
@jose-caballero jose-caballero marked this pull request as draft January 14, 2026 13:23
@jose-caballero jose-caballero force-pushed the allow_expiration_date_for_downtime branch 5 times, most recently from 1b77ab9 to 65f41ce Compare January 14, 2026 13:38
@jose-caballero jose-caballero marked this pull request as ready for review January 14, 2026 13:40
@jose-caballero jose-caballero force-pushed the allow_expiration_date_for_downtime branch from 65f41ce to 0c9f768 Compare January 16, 2026 09:56
@jose-caballero jose-caballero marked this pull request as draft January 26, 2026 08:46
@jose-caballero jose-caballero force-pushed the allow_expiration_date_for_downtime branch 2 times, most recently from 88deae8 to 231bfd6 Compare January 26, 2026 11:06
@jose-caballero jose-caballero marked this pull request as ready for review January 26, 2026 11:09
@jose-caballero jose-caballero force-pushed the allow_expiration_date_for_downtime branch from 231bfd6 to 88f8ec1 Compare January 26, 2026 11:46
In this task we address the request from this JIRA ticket:
https://stfc.atlassian.net/browse/CLDSCRM-2737

We want to allow to specify not only the number of hours
that a given hypervisor is going to be down, but also:
- the number of days
- the number of days and hours
- the exact end time

In this commit we make the following changes:
- we add a new field to the YAML file for the user form,
  and improve the description sentences
- we implement the necessary code to allow those new input formats

The new code has been implemented almost entirely as separate functions
called by the old one. These new functions return the final total number
of hours for the donwtime interval. It is true that, doing things this way,
for some cases the datetime object for the endtime is calculated twice.
The changes have been implemented this way for two reasons:
- modify the original code a little as possible,
  avoiding the need to refactor too much existing unittest code
- the new logic is implemented in separate small functions that return outputs.
  This way, we can unittest them in "black box" mode -checking only the
  inputs, outputs, and excpetions, but not the implementation-
  and still covering the entire logic

A potential refactoring in the future could be to implement the
time calculations we are introducing in this change inside the
Dowmtime classes. But I would suggest to do that, if so, only when we
do not need icinga downtimes anymore and this st2 Action requires some
cleanup and simplification
@jose-caballero jose-caballero force-pushed the allow_expiration_date_for_downtime branch from 88f8ec1 to 2abf219 Compare January 26, 2026 11:51
DavidFair
DavidFair previously approved these changes Jan 26, 2026
Copy link
Collaborator

@DavidFair DavidFair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, there's a load of duplication in the tests (mainly around get_number_of_hours_from_duration being tested after get_number_of_hours also tests the same thing) but it isn't worth holding this PR up for

Sorry it went through several iterations, but (IMO) it's in a much nicer form for future developers

Fix the old unittest as the signature for the original function has
changed a little bit.

Add tests for the new functions introduced in this task.
Copy link
Collaborator

@anish-mudaraddi anish-mudaraddi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jose-caballero jose-caballero merged commit d61ff33 into main Jan 26, 2026
12 of 14 checks passed
@jose-caballero jose-caballero deleted the allow_expiration_date_for_downtime branch January 26, 2026 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants