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

Delete invalid nil timestamp event streams #704

Merged
merged 1 commit into from
Aug 30, 2023

Conversation

jrafanie
Copy link
Member

Until ManageIQ/manageiq-providers-kubernetes#501 code lands, openshift events with nil timestamps will be added to event streams.

These are invalid events and really mess up our queries since we timestamp is a lookup field and expected to use an index which doesn't happen when it's nil.

@jrafanie jrafanie force-pushed the delete_nil_timestamp_event_streams branch 3 times, most recently from b31b299 to ba9538d Compare August 30, 2023 14:46
@jrafanie jrafanie force-pushed the delete_nil_timestamp_event_streams branch from ba9538d to e5f65aa Compare August 30, 2023 17:06

migrate

expect(event_stream_stub.pluck(:id, :timestamp).flatten).to eq([good.id, good.timestamp])
Copy link
Member

Choose a reason for hiding this comment

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

This is failing on timestamp precision,

  1) DeleteNilTimestampEventStreams#up removes nil timestamp events, leaving others
     Failure/Error: expect(event_stream_stub.pluck(:id, :timestamp).flatten).to eq([good.id, good.timestamp])
     
       expected: [4, 2023-08-30 17:33:20.160235809 +0000]
            got: [4, 2023-08-30 17:33:20.160235000 +0000]

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, I was hoping to keep it simple. I guess I can check that's not nil.

Copy link
Member

Choose a reason for hiding this comment

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

You could use .to be_within() or .to_i both

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I went with present since that's the intention of it.

Until ManageIQ/manageiq-providers-kubernetes#501 code
lands, openshift events with nil timestamps will be added to event streams.

These are invalid events and really mess up our queries since we timestamp is a lookup
field and expected to use an index which doesn't happen when it's nil.
@jrafanie jrafanie force-pushed the delete_nil_timestamp_event_streams branch from e5f65aa to 2ba5fb1 Compare August 30, 2023 20:16
@miq-bot
Copy link
Member

miq-bot commented Aug 30, 2023

Checked commit jrafanie@2ba5fb1 with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint
2 files checked, 0 offenses detected
Everything looks fine. ⭐

@agrare agrare merged commit acfb86d into ManageIQ:master Aug 30, 2023
2 checks passed
@jrafanie jrafanie deleted the delete_nil_timestamp_event_streams branch August 30, 2023 22:18
@Fryguy
Copy link
Member

Fryguy commented Sep 12, 2023

Backported to quinteros in commit 3b9e6b2.

commit 3b9e6b2bcb16fd7f71306ab3da2763000bc5cc69
Author: Adam Grare <[email protected]>
Date:   Wed Aug 30 16:22:44 2023 -0400

    Merge pull request #704 from jrafanie/delete_nil_timestamp_event_streams
    
    Delete invalid nil timestamp event streams
    
    (cherry picked from commit acfb86dc6541e382ec9b7e5972ba551f7ec8d574)

Fryguy pushed a commit that referenced this pull request Sep 12, 2023
Delete invalid nil timestamp event streams

(cherry picked from commit acfb86d)
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.

4 participants