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

feat: trigger conditions reset. Closes #1381 #1392

Merged
merged 7 commits into from
Oct 26, 2021

Conversation

whynowy
Copy link
Member

@whynowy whynowy commented Oct 22, 2021

Signed-off-by: Derek Wang [email protected]

Closes: #1381

Checklist:

@whynowy whynowy changed the title feat: trigger conditions reset feat: trigger conditions reset. Closes #1381 Oct 22, 2021
Signed-off-by: Derek Wang <[email protected]>
Signed-off-by: Derek Wang <[email protected]>
@sarabala1979 sarabala1979 self-assigned this Oct 25, 2021
@sarabala1979
Copy link
Member

sarabala1979 commented Oct 25, 2021

@whynowy Can you add examples of how to configure the resetCondition?

@whynowy
Copy link
Member Author

whynowy commented Oct 25, 2021

@whynowy Can you add examples of how to configure the resetCondition?

👍

Signed-off-by: Derek Wang <[email protected]>
@whynowy
Copy link
Member Author

whynowy commented Oct 25, 2021

Here you go @sarabala1979 .

.
Signed-off-by: Derek Wang <[email protected]>
Signed-off-by: Derek Wang <[email protected]>
Signed-off-by: Derek Wang <[email protected]>
@@ -306,10 +304,17 @@ func (n *natsStreaming) processEventSourceMsg(m *stan.Msg, msgHolder *eventSourc
return
}
if result != true {
// Log current meet dependency information
Copy link
Member

Choose a reason for hiding this comment

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

nice

@@ -18,6 +18,11 @@ spec:
- template:
# Boolean expression contains dependency names to determine whether to execute the trigger or not
conditions: "test-dep"
conditionsReset:
- byTime:
# Reset conditions at 23:59 everyday
Copy link
Member

Choose a reason for hiding this comment

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

Will condition reset below scenario?

  1. Sensor down at the scheduled time and restart after that.
  2. Sensor restart anytime

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately, if the sensor is down at the time it is scheduled, it will miss the reset, but I don't think it's worth using persistence somehow. If this is a big concern for the users, they should increase the Sensor replicas to achieve HA.

Copy link
Member

Choose a reason for hiding this comment

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

got it

@whynowy whynowy merged commit 65c49d1 into argoproj:master Oct 26, 2021
@whynowy whynowy deleted the resetconditions branch October 26, 2021 04:01
whynowy added a commit that referenced this pull request Nov 8, 2021
* feat: trigger conditions reset

Signed-off-by: Derek Wang <[email protected]>
BulkBeing pushed a commit to BulkBeing/argo-events that referenced this pull request Nov 16, 2021
* feat: trigger conditions reset

Signed-off-by: Derek Wang <[email protected]>
juliev0 pushed a commit to juliev0/argo-events that referenced this pull request Mar 29, 2022
* feat: trigger conditions reset

Signed-off-by: Derek Wang <[email protected]>
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.

Trigger conditions reset
2 participants