adding signal filters #54

magaldima · 2018-07-23T21:02:02Z

closes #26

CLAassistant · 2018-07-23T21:02:10Z

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

shrinandj · 2018-07-24T17:46:13Z

examples/time-filter-webhook.yaml

+        method: POST
+      filters:
+        time:
+          start: "2016-05-10T15:04:05Z07:00"


Does this mean that the trigger will be fired only if the signal is fired between the start and stop time?

shrinandj · 2018-07-24T17:46:46Z

pkg/apis/sensor/v1alpha1/types.go

+// DataFilter describes constraints and filters for event data
+// Regular Expressions are purposefully not a feature as they are overkill for our uses here
+// See Rob Pike's Post: https://commandcenter.blogspot.com/2011/08/regular-expressions-in-lexing-and.html
+type DataFilter struct {


Can you add an example for DataFilter as well?

Never mind.. Just saw it.

lemme know if I have enough examples

shrinandj · 2018-07-24T17:49:29Z

controller/signal-filter_test.go

@@ -0,0 +1,316 @@
+package controller


Great job on adding unit tests! :-)

shrinandj

Looks great!

magaldima · 2018-07-24T19:29:41Z

These generated conflicts are annoying.. are these a feature of the

[[constraint]]
  name = "k8s.io/code-generator"
  branch = "release-1.10"

dependency?

Anyway, do you know if it matters how I resolve these conflicts?

Also side note, every time I now do make codegen I have to manually delete some fake Discovery code that you pointed out in #51 . Do you think we should raise an issue in the kubernetes repo with this?

Created #56 for this.

magaldima · 2018-07-24T21:13:58Z

@shrinandj rebased on top of master, can you re-approve?

magaldima requested review from plasma86, shrinandj and spk83 July 23, 2018 21:02

shrinandj reviewed Jul 24, 2018

View reviewed changes

controller/signal-filter_test.go

@@ -0,0 +1,316 @@

package controller

Copy link

Contributor

shrinandj Jul 24, 2018

Choose a reason for hiding this comment

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

Great job on adding unit tests! :-)

shrinandj previously approved these changes Jul 24, 2018

View reviewed changes

adding signal filters

e1604cb

magaldima dismissed shrinandj’s stale review via e1604cb July 24, 2018 21:13

magaldima force-pushed the add-signal-filters branch from 5a04f39 to e1604cb Compare July 24, 2018 21:13

shrinandj approved these changes Jul 24, 2018

View reviewed changes

magaldima merged commit 5124282 into master Jul 24, 2018

magaldima deleted the add-signal-filters branch July 25, 2018 12:53

juliev0 pushed a commit to juliev0/argo-events that referenced this pull request Mar 29, 2022

adding signal filters (argoproj#54)

57c114d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adding signal filters #54

adding signal filters #54

magaldima commented Jul 23, 2018

CLAassistant commented Jul 23, 2018 •

edited

Loading

shrinandj Jul 24, 2018

magaldima Jul 24, 2018

shrinandj Jul 24, 2018

shrinandj Jul 24, 2018

magaldima Jul 24, 2018

shrinandj Jul 24, 2018

shrinandj left a comment

magaldima commented Jul 24, 2018 •

edited

Loading

magaldima commented Jul 24, 2018

adding signal filters #54

adding signal filters #54

Conversation

magaldima commented Jul 23, 2018

CLAassistant commented Jul 23, 2018 • edited Loading

shrinandj Jul 24, 2018

Choose a reason for hiding this comment

magaldima Jul 24, 2018

Choose a reason for hiding this comment

shrinandj Jul 24, 2018

Choose a reason for hiding this comment

shrinandj Jul 24, 2018

Choose a reason for hiding this comment

magaldima Jul 24, 2018

Choose a reason for hiding this comment

shrinandj Jul 24, 2018

Choose a reason for hiding this comment

shrinandj left a comment

Choose a reason for hiding this comment

magaldima commented Jul 24, 2018 • edited Loading

magaldima commented Jul 24, 2018

CLAassistant commented Jul 23, 2018 •

edited

Loading

magaldima commented Jul 24, 2018 •

edited

Loading