Deprecate iso8601 public API, and some other cleanups#4097
Draft
nrwahl2 wants to merge 99 commits into
Draft
Conversation
nrwahl2
force-pushed
the
nrwahl2-time
branch
2 times, most recently
from
d9b26ea to
f6540ec
Compare
nrwahl2
changed the title
Contributor
Whew, yeah that's a lot of commits. Go ahead and split it up (also looks like you'll need to resolve a conflict) so it's not quite so demoralizing to review. |
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To match header. Also rename lpc to i in crm_time_add_months(). Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
The caller already skips 'T' characters. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
This makes crm_time_parse_duration() a little bit less convoluted. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
It's clearer to show the original string value than the parsed integer. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
It's incorrect to say that a particular integer is too large or too small, in the way we were doing. In the lax way we're parsing durations, multiple elements with the same designator may occur. Also, elements with different designators can add to the same field of the crm_time_t object. So multiple elements, none of which are too large or too small on their own, can sum to a value that overflows the size of an int. Reduce duplication and simplify the log messages on overflow. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
There's no "right" way to do this, and it should be exceedingly rare that our users specify years or months as part of a duration. But since we unfortunately support a ISO 8601 duration-like syntax in a wide variety of contexts, we should handle it sanely. Hopefully we can deprecate and drop it sometime. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
The pcmk__time_valid_year() function will be used outside this file soon. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
The reason for the deprecation warning regardless of installed version, is that we define GLIB_VERSION_MIN_REQUIRED and GLIB_VERSION_MAX_ALLOWED as GLIB_VERSION_2_42. So we are strictly bound to the 2.42 API. However, when we can use version 2.58 or later, we're better off using g_time_zone_new_offset(). This avoids the need to convert the offset to a string. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Allocate a new string for readability, even though we know the exact string length in advance and were previously taking advantage of that in order to use a static buffer. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
This isn't really in keeping with the function name. However, some callers care only about the resulting hours and minutes, so this is more convenient than requiring those callers to pass dummy seconds output variables. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
crm_time_new(NULL) always returns non-NULL. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Check whether parsing was successful, not whether the value was 0 (which is technically a valid value: epoch). Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
The spec with year 10000 is valid for now. Note that years get scanned as uint32_t, so -1 becomes UINT32_MAX. I expect that number to be platform-independent. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
...for the start and end of a period. This is a behavioral change but shouldn't affect any reasonable cluster configurations. Use of the iso8601 CLI tool or of the crm_time_parse_period() function directly will be affected, for dates with year >= 10000. ISO 8601 only guarantees support for years up to four digits. Year 0 is treated as 1 BCE. However, we consider year 0 invalid. The goal is to move to using GLib's GDateTime internally. 0001-01-1 to 9999-12-31 is the supported range for GDateTime. 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's equivalent to free(). There are no dynamically allocated fields within crm_time_t. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Use free() instead. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To replace pcmk_copy_time(). Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
External callers have no need to copy 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_add(). Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
External callers have no need to add to 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_subtract(). Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
External callers have no need to subtract from 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_add_seconds(). Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
External callers have no need to add seconds to a crm_time_t object. Pacemaker should not be used for general-purpose date/time manipulation. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
External callers have no need to add minutes to a crm_time_t object. Pacemaker should not be used for general-purpose date/time manipulation. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
External callers have no need to add hours to 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_add_days(). Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
External callers have no need to add days to a crm_time_t object. Pacemaker should not be used for general-purpose date/time manipulation. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
External callers have no need to add weeks to a crm_time_t object. Pacemaker should not be used for general-purpose date/time manipulation. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
External callers have no need to add months to 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_add_years(). Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
External callers have no need to add years to 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>
To replace pcmk_evaluate_rule(). Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
There doesn't seem to be much use case for this. We could come up with a contrived use case: a user wants to find out whether a rule will evaluate to true or false under some combination of current date/time, node attributes, resource parameters, etc. The user might want to test a rule that's already in the configuration or one that they're considering adding. However, we have pcmk_check_rule() and pcmk_check_rules(), which serve a similar but more limited purpose. Given an input CIB, a date/time object, and one or more rule IDs (for rules in the input CIB), those functions determine whether the rules woudl evaluate to true or false at the given date/time. Those functions can replace arbitrary calls to pcmk_evaluate_rule(). Instead of setting up your own pcmk_rule_input_t object for pcmk_evaluate_rule(), create the input CIB so that it contains the desired node attributes, resource parameters, etc. Then pass that input CIB and a date/time object to pcmk_check_rule(s)(). Some of the documentation for the pcmk_rule_input_t struct is a bit vague for external users. More importantly, it will be nice to be able to change the implementation without affecting any external users. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To replace pcmk_rule_input_t. Also add a conversion function, so that we can change the internal pcmk__rule_input_t definition later. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
This requires changing the helper functions too, some of which are non-static. However, doing this for pcmk__evaluate_rule() and all its helpers at the same time, turned out to be cleaner than doing them one by one. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Instead of pcmk_rule_input_t. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Instead of pcmk_rule_input_t. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Instead of pcmk_rule_input_t. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
It's only used by deprecated functions now. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
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.
I've been doing some work in the background on switching over from
crm_time_ttoGDateTime. It's a pretty big pain, but that's largely becauseGDateTimeonly supports dates between0001-01-01 00:00:00and9999-12-31 23:59:59. We're going to have to make behavioral changes in some cases, to consider dates invalid if they can't be represented by four digits, and/or to clip them to the valid range.One commit in this PR does this for
crm_time_period_t. It is odd to change behavior in that function immediately before deprecating it. However, the more places we can guarantee acrm_time_tis in the 0001 to 9999 range, the easier future changes will be.Another does this for
crm_rule.This PR pulls out the commits I have so far that don't involve
GDateTimeorGDatedirectly.Edit: I realized we can deprecate almost all of the public API at this point, so I went ahead and did that. The PR is now much larger, but most of the commits are trivial. We can split it up if you want.