Deprecate crm_time_period_t, and some other cleanups#4105
Open
nrwahl2 wants to merge 29 commits intoClusterLabs:mainfrom
Open
Deprecate crm_time_period_t, and some other cleanups#4105nrwahl2 wants to merge 29 commits intoClusterLabs:mainfrom
nrwahl2 wants to merge 29 commits intoClusterLabs:mainfrom
Conversation
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>
Instead, pass the start and end fields. This is an incremental step toward getting rid of crm_time_period_t. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
This copies the crm_time_parse_period() logic to a static function in
the tool. The plan is to deprecate the public API function and the data
type itself.
This temporarily exposes valid_time() internally without changing the
name. The plan is to pull pcmk__time_valid_year() into the validity
check soon. This also temporarily defines {BEGIN,END}_VALID_RANGE_S in
tools/iso8601.c.
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
(Note: we've often been using the term "period" to refer to an ISO 8601 interval.) There doesn't seem to be a very good use case for this function. Perhaps after querying a time period/interval XML attribute from the CIB, a user could parse it with this function. But a scenario where that's useful seems pretty contrived. It appears we don't use ISO 8601 intervals for any piece of the configuration or for any implementation detail. pcmk_parse_interval_spec() actually parses a duration, not an interval. The implementation is also incomplete and incorrect. So I see no reason to keep this. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
There is no longer a constructor for crm_time_period_t and no reason to create one. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
ISO 8601 guarantees support only for years in this range (representable by four digits). We want to move to using GDateTime, which requires dates to be in this range. 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
1 participant
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.These commits don't involve
GDateTimeorGDatedirectly.This is the first piece from #4097.