Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query metric rollups using start_date derived from the first rollup #1265

Conversation

jrafanie
Copy link
Member

@jrafanie jrafanie commented Aug 15, 2024

This resolves a time based sporadic failure. This means it works for many hours of the day but fails with specific ones.

Previously, Time.zone is UTC, so Time.zone.today when run at August 15th at 12:05 AM UTC would return Thu, 15 Aug 2024.

If we created the rollups using 1.hour.ago, the first one would be August 14th 11:05 PM.

If we our rollups were on August 14th and we're looking for August 15th hourly rollups, we find nothing.

This change ensures both the timestamp on the rollups and the start_date we query the rollups with come from the same time.

This resolves a time based sporadic failure.  This means it works for many
hours of the day but fails with specific ones.

Previously, Time.zone is UTC, so Time.zone.today when run at
August 15th at 12:05 AM UTC would return Thu, 15 Aug 2024.

If we created the rollups using 1.hour.ago, the first one would be
August 14th 11:05 PM.

If we our rollups were on August 14th and we're looking for August 15th hourly
rollups, we find nothing.

This change ensures both the timestamp on the rollups and the start_date we
query the rollups with come from the same time.
@Fryguy
Copy link
Member

Fryguy commented Aug 15, 2024

Cool I thought this was gonna involve some timecop shenanigans

@Fryguy
Copy link
Member

Fryguy commented Aug 15, 2024

So is this technically a Rails 7 issue? That is you said Time.zone previously returns UTC, so does it not now?

@miq-bot
Copy link
Member

miq-bot commented Aug 15, 2024

Checked commit jrafanie@cef1b13 with ruby 3.1.5, rubocop 1.56.3, haml-lint 0.51.0, and yamllint
1 file checked, 4 offenses detected

spec/requests/providers_spec.rb

@jrafanie
Copy link
Member Author

jrafanie commented Aug 15, 2024

So is this technically a Rails 7 issue? That is you said Time.zone previously returns UTC, so does it not now?

Sorry, "previously" meant "before this commit". Our schedule runs for CI are at 12AM UTC so 1.hour.ago is the prior day.

It looks like it's been failing, before rails 7, via scheduled but working on merges or manual runs because it's not in that 12-1AM UTC date range.

image

@Fryguy Fryguy merged commit c46f881 into ManageIQ:master Aug 16, 2024
4 checks passed
@Fryguy
Copy link
Member

Fryguy commented Aug 16, 2024

Backported to radjabov in commit 28df3ee.

commit 28df3ee4464fc32fc5294ef728f2d89cd3292046
Author: Jason Frey <fryguy9@gmail.com>
Date:   Fri Aug 16 09:19:56 2024 -0400

    Merge pull request #1265 from jrafanie/fix_sporadic_test_failures_due_to_date_wrapping
    
    Query metric rollups using start_date derived from the first rollup
    
    (cherry picked from commit c46f88165299e31dd7b73f3d94f90dec339c5b1b)

Fryguy added a commit that referenced this pull request Aug 16, 2024
…_to_date_wrapping

Query metric rollups using start_date derived from the first rollup

(cherry picked from commit c46f881)
@jrafanie jrafanie deleted the fix_sporadic_test_failures_due_to_date_wrapping branch August 16, 2024 13:22
@Fryguy Fryguy added the bug label Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants