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

Support for passing complete event payload from signal to trigger #94

Merged
merged 1 commit into from
Oct 7, 2018
Merged

Support for passing complete event payload from signal to trigger #94

merged 1 commit into from
Oct 7, 2018

Conversation

VaibhavPage
Copy link
Contributor

No description provided.

@VaibhavPage VaibhavPage requested a review from magaldima October 3, 2018 22:34
@VaibhavPage VaibhavPage changed the title Support for passing complete event payload from signal to trigger WIP: Support for passing complete event payload from signal to trigger Oct 4, 2018
@VaibhavPage VaibhavPage changed the title WIP: Support for passing complete event payload from signal to trigger Support for passing complete event payload from signal to trigger Oct 4, 2018
magaldima
magaldima suggested changes Oct 5, 2018
View reviewed changes
Copy link
Contributor

@magaldima magaldima left a comment

Choose a reason for hiding this comment

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

left some comments for some minor changes

controllers/sensor/signal-filter.go Outdated
if int(currentT.Day()) < 10 {
currentDay = "0" + currentDay
}
currentTStr := fmt.Sprintf("%d-%s-%s", currentT.Year(), currentMonth, currentDay)
Copy link
Contributor

Choose a reason for hiding this comment

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

this may be a cleaner solution: https://stackoverflow.com/questions/23428033/losing-0-for-single-digit-hours-with-golang-time-package

controllers/sensor/signal-filter_test.go Outdated
currentTStr := fmt.Sprintf("%d-%s-%d", currentT.Year(), currentMonth, currentT.Day())
currentDay := fmt.Sprintf("%d", int(currentT.Day()))
if int(currentT.Day()) < 10 {
currentDay = "0" + currentDay
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above.

controllers/sensor/validate.go Outdated
@@ -84,7 +84,15 @@ func validateSignalFilter(filter v1alpha1.SignalFilter) error {

func validateSignalTimeFilter(tFilter *v1alpha1.TimeFilter) error {
currentT := metav1.Time{Time: time.Now().UTC()}
currentTStr := fmt.Sprintf("%d-%d-%d", currentT.Year(), int(currentT.Month()), currentT.Day())
currentMonth := fmt.Sprintf("%d", int(currentT.Month()))
if int(currentT.Month()) < 10 {
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

docs/artifact-guide.md
This is a guide for help in utilizing artifacts within Argo Events. Sensors use artifacts for two purposes:
1. Object notifications for use in `Artifact` signals. (currently S3 bucket notifications are only supported)
2. A Resource Object store for use in `Resource` triggers
This is a guide for help in utilizing artifacts within Argo Events. Sensors use artifacts for Resource Object store for use in `Resource` triggers
Copy link
Contributor

Choose a reason for hiding this comment

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

and this is no longer for object notifications because of the minio listenBucketNotifications implementation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup

docs/custom-gateway.md Outdated Show resolved Hide resolved
docs/gateway-guide.md Outdated Show resolved Hide resolved
docs/index.md Outdated
@@ -3,8 +3,8 @@
## Why Argo Events?
- Containers. Designed from the ground-up as Kubernetes-native.
- Extremely lightweight. All gateways, with exception of calendar based gateway, are event-driven, meaning there is no polling involved.
- Configurable. Select gateways you want to support, deploy those to Kubernetes and configure them on the fly
- Extensible. Write custom gateways that cater to your business use cases in any language of your choice
- Configurable. Configure gateways at the runtime
Copy link
Contributor

Choose a reason for hiding this comment

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

remove "the"

docs/index.md Outdated Show resolved Hide resolved
@VaibhavPage VaibhavPage changed the title Support for passing complete event payload from signal to trigger WIP: Support for passing complete event payload from signal to trigger Oct 5, 2018
magaldima
magaldima reviewed Oct 5, 2018
View reviewed changes
examples/gateways/resource.yaml Outdated
@@ -9,7 +9,7 @@ spec:
deploySpec:
containers:
- name: "resource-events"
image: "argoproj/resource-gateway"
image: "metalgearsolid/resource-gateway"
Copy link
Contributor

Choose a reason for hiding this comment

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

is this supposed to be this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops.. updated the image

@magaldima
Copy link
Contributor

the argo-ci build & test is failing.. should we be concerned? I think we have to update the default make command to get the build to work - without docker.

test logs:

# github.com/argoproj/argo-events/controllers/sensor
controllers/sensor/validate.go:25:2: imported and not used: "github.com/argoproj/argo-events/vendor/k8s.io/apimachinery/pkg/api
s/meta/v1" as metav1
controllers/sensor/validate.go:108:30: currentT.Time undefined (type time.Time has no field or method Time)
# github.com/argoproj/argo-events/controllers/sensor
controllers/sensor/validate.go:25:2: imported and not used: "github.com/argoproj/argo-events/vendor/k8s.io/apimachinery/pkg/api
s/meta/v1" as metav1
controllers/sensor/validate.go:108:30: currentT.Time undefined (type time.Time has no field or method Time)
?       github.com/argoproj/argo-events/cmd/controllers/gateway [no test files]
?       github.com/argoproj/argo-events/cmd/controllers/gateway/transform/http  [no test files]
Makefile:237: recipe for target 'test' failed
make: *** [test] Error 2

@VaibhavPage
Copy link
Contributor Author

fixed the build problem and all tests pass locally. Its weird that namespace_test is failing in Argo CI

@VaibhavPage VaibhavPage changed the title WIP: Support for passing complete event payload from signal to trigger Support for passing complete event payload from signal to trigger Oct 6, 2018
@VaibhavPage VaibhavPage merged commit 9acb556 into master Oct 7, 2018
@VaibhavPage VaibhavPage deleted the trigger-param-fix branch October 28, 2018 12:01
juliev0 pushed a commit to juliev0/argo-events that referenced this pull request Mar 29, 2022
@VaibhavPage
Added support for passing complete event payload from signal to trigg…
feb7330 
…er. Updated docs (argoproj#94)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@magaldima magaldima magaldima 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.
2 participants
@VaibhavPage @magaldima