Skip to content

analyze: fix (unexpected) timestamp parsing#4347

Merged
holmanb merged 1 commit into
canonical:mainfrom
igalic:fix/gnu-date-timestamp-parsing-tests
Aug 16, 2023
Merged

analyze: fix (unexpected) timestamp parsing#4347
holmanb merged 1 commit into
canonical:mainfrom
igalic:fix/gnu-date-timestamp-parsing-tests

Conversation

@igalic
Copy link
Copy Markdown
Collaborator

@igalic igalic commented Aug 14, 2023

Proposed Commit Message

analyze: fix (unexpected) timestamp parsing

In case of very unexpected timestamps, we pass them on to date.
Unfortunately, only GNU date is able to perform this level of deduction
(guess work).

Rework the code and tests to look for GNU date on non-Linux platforms.
Only fail, and loudly! if GNU date cannot be found.

This patch (partially)

fixes GH-4333

Sponsored by: The FreeBSD Foundation

Additional Context

Test Steps

Checklist:

  • My code follows the process laid out in the documentation
  • I have updated or added any unit tests accordingly
  • I have updated or added any documentation accordingly

@igalic igalic force-pushed the fix/gnu-date-timestamp-parsing-tests branch 2 times, most recently from f7ce6ba to e9180dc Compare August 15, 2023 09:19
In case of very unexpected timestamps, we pass them on to date.
Unfortunately, only GNU date is able to perform this level of deduction
(guess work).

Rework the code and tests to look for GNU date on non-Linux platforms.
Only fail, by throwing an Exception! if GNU date cannot be found.
add `timestampstr` to that exception, so we know why we're in this code
path to begin with.

Sponsored by: The FreeBSD Foundation

This patch (partially)

Fixes: canonicalGH-4333

Co-authored-by: Brett Holman <brett.holman@canonical.com>
Copy link
Copy Markdown
Collaborator Author

@igalic igalic left a comment

Choose a reason for hiding this comment

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

thoughts?

Comment thread cloudinit/analyze/dump.py Outdated
Comment thread cloudinit/analyze/dump.py Outdated
Copy link
Copy Markdown
Member

@holmanb holmanb left a comment

Choose a reason for hiding this comment

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

Aside from "making it just work" (which this PR does), I'm tempted to say that maybe it would be better if at some point in the future we didn't depend on GNU date and transitionally logged a warning anytime parse_timestamp_from_date() is called so that we can collect logs for any formats that could be handled in python code directly. I didn't see this path actually taken on Ubuntu, so I'm curious when (if) this path actually gets taken.

Thoughts?

@igalic
Copy link
Copy Markdown
Collaborator Author

igalic commented Aug 16, 2023

I've only seen this in tests.
I've never seen it hit an exception in real life

@holmanb
Copy link
Copy Markdown
Member

holmanb commented Aug 16, 2023

I've only seen this in tests. I've never seen it hit an exception in real life

I'll make it a separate bug.

@igalic igalic force-pushed the fix/gnu-date-timestamp-parsing-tests branch from fcfaa60 to 72b78e2 Compare August 16, 2023 12:20
@holmanb holmanb merged commit 4f09548 into canonical:main Aug 16, 2023
@igalic igalic deleted the fix/gnu-date-timestamp-parsing-tests branch August 16, 2023 17:31
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.

2 participants