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.

Sign up for GitHub

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

Jump to bottom

fix: changing lastResetTime to a time.Time from int64 seconds so time comparison can occur at a finer granularity #1695

Merged
merged 2 commits into from
Mar 2, 2022
Merged

fix: changing lastResetTime to a time.Time from int64 seconds so time comparison can occur at a finer granularity #1695

merged 2 commits into from
Mar 2, 2022

Conversation

juliev0
Copy link
Contributor

@juliev0 juliev0 commented Mar 2, 2022

Checklist:

@juliev0
changing lastResetTime to a time.Time from int64 seconds so time comp…
ac2b9c5 
…arison can occur at a finer granularity

Signed-off-by: Julie Vogelman <julie_vogelman@intuit.com>
@whynowy whynowy changed the title changing lastResetTime to a time.Time from int64 seconds so time comparison can occur at a finer granularity fix: changing lastResetTime to a time.Time from int64 seconds so time comparison can occur at a finer granularity Mar 2, 2022
@juliev0
dummy commit to re-trigger build
b809ba2 
Signed-off-by: Julie Vogelman <julie_vogelman@intuit.com>
@juliev0
Copy link
Contributor Author

juliev0 commented Mar 2, 2022

This is in reference to this bug: #1694

@juliev0
Copy link
Contributor Author

juliev0 commented Mar 2, 2022

I have tested this with the following conditions:

  1. similar to bug: just one dependency in the condition and multiple webhooks curled immediately one after the other: verifying each is triggered in sensor
  2. conditional expression involving && and both webhooks are curled - verify sensor is triggered
  3. standard condition reset case with conditional expression involving &&: verifying condition reset occurs
  4. condition reset case where pod is down at the cron time

whynowy
whynowy approved these changes Mar 2, 2022
View reviewed changes
Copy link
Member

@whynowy whynowy left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the quick fix!

@whynowy whynowy merged commit 35f643a into argoproj:master Mar 2, 2022
@tooptoop4
Copy link
Contributor

when is next release with this fix?

whynowy pushed a commit that referenced this pull request Mar 2, 2022
@juliev0 @whynowy
fix: changing lastResetTime to a time.Time from int64 seconds so time…
1c6a5d4 
… comparison can occur at a finer granularity (#1695)

* changing lastResetTime to a time.Time from int64 seconds so time comparison can occur at a finer granularity

Signed-off-by: Julie Vogelman <julie_vogelman@intuit.com>
BulkBeing pushed a commit to BulkBeing/argo-events that referenced this pull request Mar 7, 2022
@juliev0 @BulkBeing
fix: changing lastResetTime to a time.Time from int64 seconds so time…
9d19f3d 
… comparison can occur at a finer granularity (argoproj#1695)

* changing lastResetTime to a time.Time from int64 seconds so time comparison can occur at a finer granularity

Signed-off-by: Julie Vogelman <julie_vogelman@intuit.com>
juliev0 added a commit to juliev0/argo-events that referenced this pull request Mar 29, 2022
@juliev0
fix: changing lastResetTime to a time.Time from int64 seconds so time…
8a11839 
… comparison can occur at a finer granularity (argoproj#1695)

* changing lastResetTime to a time.Time from int64 seconds so time comparison can occur at a finer granularity

Signed-off-by: Julie Vogelman <julie_vogelman@intuit.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@whynowy whynowy whynowy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@juliev0 @tooptoop4 @whynowy