Deprecate crm_time_calculate_duration() through crm_time_get_seconds()#4130
Open
nrwahl2 wants to merge 47 commits into
Open
Deprecate crm_time_calculate_duration() through crm_time_get_seconds()#4130nrwahl2 wants to merge 47 commits into
nrwahl2 wants to merge 47 commits into
Conversation
Pacemaker should not be used for general-purpose date/time manipulation. There's no need to calculate a duration (as the difference between two crm_time_t objects) for the purpose of administering Pacemaker. Where Pacemaker uses durations, it parses them directly from strings. A crm_time_t object can be displayed as a string if desired, and then a user can compare two strings through whatever method they desire. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
nrwahl2
commented
Jun 22, 2026
To replace crm_time_parse_duration(). Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Pacemaker parses durations from strings in a few places. However, there is no need for external code to do this for the purpose of administering Pacemaker. Pacemaker should not be used for general-purpose date/time manipulation. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Add a couple of missing includes elsewhere, since they break otherwise. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Use crm_time_new() to create a new crm_time_t object from a string (or NULL to use the current time). There is no need for external callers to create an uninitialized object and then set the fields (such as years) later. Pacemaker should not be used for general-purpose date/time manipulation. pcmk_evaluate_rule() can take an uninitialized (undefined) crm_time_t object as the next_change argument and update it as appropriate. However, that argument is for use by the scheduler, not for external callers. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
nrwahl2
force-pushed
the
nrwahl2-time_first
branch
from
3bb2fcd to
9629516
Compare
clumens
reviewed
Jul 1, 2026
| @@ -1,110 +0,0 @@ | |||
| /* | |||
Contributor
There was a problem hiding this comment.
This should be added to .gitignore, too.
Contributor
Author
There was a problem hiding this comment.
/lib/*/tests/*/*_test is already in .gitignore. Is there a reason why this *_test.c file should be there?
| return pcmk__time_get_seconds(dt); | ||
| } | ||
|
|
||
| #define EPOCH_SECONDS 62135596800ULL // Calculated using pcmk__time_get_seconds |
Contributor
There was a problem hiding this comment.
What does this magic constant represent?
Contributor
Author
There was a problem hiding this comment.
Seconds between 0001-01-01T00:00:00 and 1970-01-01T00:00:00, as computed by pcmk__time_get_seconds()
To replace crm_time_is_defined(). Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
External callers should always be using crm_time_new() to create a
crm_time_t object. Such objects will be initialized ("defined").
Pacemaker does not return crm_time_t objects via public API functions,
except as an output argument of pcmk_evaluate_rule(). However, that
argument is for use by the scheduler, not for external callers.
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To replace crm_time_as_string(). Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Pacemaker should not be used for general-purpose date/time manipulation. It also does not return crm_time_t objects (except in the case of pcmk_evaluate_rule(), as discussed in previous commit messages). So external callers have no need to convert a crm_time_t to a string representation. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To replace crm_time_log_date. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To replace crm_time_log_timeofday. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To replace crm_time_log_with_timezone. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Improve variable names and compare explicitly against zero where appropriate. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
This slightly changes behavior if seconds is zero and usec is negative (see sec_usec_as_string()). But as mentioned in the note within duration_as_string()'s Doxygen, the duration formatting is already incorrect when any fields are negative. sec_usec_as_string() calls QB_ABS(usec), so having this extra call was a bit confusing. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To replace crm_time_log_duration. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To replace crm_time_ordinal. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To replace crm_time_weeks. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To replace crm_time_seconds. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To replace crm_time_epoch. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To replace crm_time_usecs. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To replace crm_time_compare(). Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
It's always TRUE. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To replace crm_time_get_timeofday(). Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
External callers have no need to inspect a crm_time_t object. Pacemaker should not be used for general-purpose date/time manipulation. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
It always returns TRUE. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To replace crm_time_get_gregorian(). Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
External callers have no need to inspect a crm_time_t object. Pacemaker should not be used for general-purpose date/time manipulation. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
External callers have no need to inspect a crm_time_t object. Pacemaker should not be used for general-purpose date/time manipulation. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To replace crm_time_get_seconds(). Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
External callers have no need to inspect a crm_time_t object. Pacemaker should not be used for general-purpose date/time manipulation. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
We used these in order to be explicit about operator precedence for clarity. However, I now believe these particular parentheses add more clutter than clarity. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Make it more similar to pcmk__compare_versions(), pcmk__compare_node_capacities(), etc. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
nrwahl2
added
waiting for author
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Also add a couple of FIXME comments for incorrect logic. Since public API functions call pcmk__time_get_ymd(), we'll need to preserve behavior for those functions. However, we can write a correct function body for internal use soon. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
nrwahl2
force-pushed
the
nrwahl2-time_first
branch
from
9629516 to
b34bbf5
Compare
nrwahl2
added
needs attention
Contributor
Author
|
Updated to address review. Most of the changes are in new commits on top. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is the next batch from #4097.