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

fix wrong timestamp format #338

Merged

Conversation

AlexanderZagaynov
Copy link
Contributor

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1724312

Root issue was the wrond timestamp format in the first query to the Azure API since worker start.
Another potential issue could happen during that worker restart - if it lasts more than one minute. I extended it to 5 minutes and added a check to avoid duplicated events.

@AlexanderZagaynov
Copy link
Contributor Author

@agrare @juliancheal please review

@juliancheal
Copy link
Member

👍 LTGM.

Copy link
Member

@agrare agrare left a comment

Choose a reason for hiding this comment

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

Hey @AlexanderZagaynov we are trying to get away from event catchers having direct access to the database with the goal of moving these types of workers away from rails to reduce memory usage.

Handling duplicate events could probably be done in the Event Handler so it would apply to all providers.

Could we just move to the format_timestamp(1.minute.ago) to fix the original issue?

@miq-bot
Copy link
Member

miq-bot commented Jul 24, 2019

Checked commits AlexanderZagaynov/manageiq-providers-azure@d5367d0~...31f7a6e with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🍪

@AlexanderZagaynov
Copy link
Contributor Author

@agrare done :)

@AlexanderZagaynov AlexanderZagaynov changed the title Do not lose events during worker restart fix wrong timestamp format Jul 24, 2019
@agrare agrare added the bug label Jul 24, 2019
Copy link
Member

@agrare agrare left a comment

Choose a reason for hiding this comment

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

👍 thanks Alex!

@agrare agrare merged commit 6b90aaa into ManageIQ:master Jul 24, 2019
@agrare agrare added this to the Sprint 117 Ending Aug 5, 2019 milestone Jul 24, 2019
@simaishi
Copy link
Contributor

ivanchuk/yes and hammer/yes ?

@agrare
Copy link
Member

agrare commented Jul 25, 2019

Yes to both

simaishi pushed a commit that referenced this pull request Jul 25, 2019
@simaishi
Copy link
Contributor

Ivanchuk backport details:

$ git log -1
commit 8ebfb8ade2cbc8051dd6c357ff19644df23adf43
Author: Adam Grare <agrare@redhat.com>
Date:   Wed Jul 24 12:03:45 2019 -0400

    Merge pull request #338 from AlexanderZagaynov/BZ-1724312_unable_to_receive_events
    
    fix wrong timestamp format
    
    (cherry picked from commit 6b90aaac83e228a4fa52a31123dadf43f8ce42e2)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1724312

simaishi pushed a commit that referenced this pull request Jul 25, 2019
@simaishi
Copy link
Contributor

Hammer backport details:

$ git log -1
commit 6195bd3c428cabfe0290eea54b55441496e9721b
Author: Adam Grare <agrare@redhat.com>
Date:   Wed Jul 24 12:03:45 2019 -0400

    Merge pull request #338 from AlexanderZagaynov/BZ-1724312_unable_to_receive_events
    
    fix wrong timestamp format
    
    (cherry picked from commit 6b90aaac83e228a4fa52a31123dadf43f8ce42e2)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1733383

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.

None yet

6 participants