Deprecate iso8601 public API, and some other cleanups#4097
Draft
nrwahl2 wants to merge 91 commits intoClusterLabs:mainfrom
Draft
Deprecate iso8601 public API, and some other cleanups#4097nrwahl2 wants to merge 91 commits intoClusterLabs:mainfrom
nrwahl2 wants to merge 91 commits intoClusterLabs:mainfrom
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 <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
To match header. Also rename lpc to i in crm_time_add_months(). Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
The caller already skips 'T' characters. Signed-off-by: Reid Wahl <[email protected]>
This makes crm_time_parse_duration() a little bit less convoluted. Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
It's clearer to show the original string value than the parsed integer. Signed-off-by: Reid Wahl <[email protected]>
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 <[email protected]>
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 <[email protected]>
The pcmk__time_valid_year() function will be used outside this file soon. Signed-off-by: Reid Wahl <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
crm_time_new(NULL) always returns non-NULL. Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Check whether parsing was successful, not whether the value was 0 (which is technically a valid value: epoch). Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
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 <[email protected]>
...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 <[email protected]>
To replace crm_time_get_gregorian(). Signed-off-by: Reid Wahl <[email protected]>
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 <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
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 <[email protected]>
To replace crm_time_get_seconds(). Signed-off-by: Reid Wahl <[email protected]>
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 <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
To replace crm_time_get_seconds_since_epoch(). The name "pcmk__time_to_unix()" is chosen in order to align with g_date_time_to_unix(), which we plan to adopt eventually. Signed-off-by: Reid Wahl <[email protected]>
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 <[email protected]>
It's equivalent to free(). There are no dynamically allocated fields within crm_time_t. Signed-off-by: Reid Wahl <[email protected]>
Use free() instead. Signed-off-by: Reid Wahl <[email protected]>
To replace pcmk_copy_time(). Signed-off-by: Reid Wahl <[email protected]>
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 <[email protected]>
To replace crm_time_add(). Signed-off-by: Reid Wahl <[email protected]>
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 <[email protected]>
To replace crm_time_subtract(). Signed-off-by: Reid Wahl <[email protected]>
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 <[email protected]>
To replace crm_time_add_seconds(). Signed-off-by: Reid Wahl <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
To replace crm_time_add_days(). Signed-off-by: Reid Wahl <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
To replace crm_time_add_years(). Signed-off-by: Reid Wahl <[email protected]>
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 <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
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.