passing entire s3 notification payload as event data #78
Conversation
controller/config.go
Outdated
| @@ -31,7 +32,7 @@ import ( | |||
| ) | |||
|
|
|||
| func (c *SensorController) watchControllerConfigMap(ctx context.Context) (cache.Controller, error) { | |||
| c.log.Info("watching sensor controller config map updates") | |||
| log.Info("watching sensor controller config map updates") | |||
There was a problem hiding this comment.
Can you rebase off of master again; now that the logging changes have been merged?
|
fixed the conflicts |
magaldima
force-pushed
the
make-s3-notifications-the-event-data
branch
from
a29644e
to
118eec5
Compare
|
@shrinandj can you re-approve this? |
| @@ -466,7 +466,7 @@ type ArtifactLocation struct { | |||
|
|
|||
| // S3Artifact contains information about an artifact in S3 | |||
| type S3Artifact struct { | |||
| S3Bucket `json:",inline" protobuf:"bytes,4,opt,name=s3Bucket"` | |||
| S3Bucket `json:",inline"` | |||
There was a problem hiding this comment.
I don't understand protobufs yet... Why is this change needed?
There was a problem hiding this comment.
I noticed this when I ran make codegen.. seems to be related to hack/generate-proto.sh where it overrides the types.go file's protobuf annotations. I'm definitely not a protobuf expert, but I deduced a pattern when defining the API:
When you have an inline field, you don't have a protobuf annotation for it. For example:
// Sensor is the definition of a sensor resource
// +genclient
// +genclient:noStatus
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
type Sensor struct {
v1.TypeMeta `json:",inline"`
v1.ObjectMeta `json:"metadata" protobuf:"bytes,1,opt,name=metadata"`
Spec SensorSpec `json:"spec" protobuf:"bytes,2,opt,name=spec"`
Status SensorStatus `json:"status" protobuf:"bytes,3,opt,name=status"`
}
However, when I defined an inlined type and then ran make codegen it added a protobuf annotation where I originally didn't have one:
// ArtifactSignal describes an external object dependency
type ArtifactSignal struct {
ArtifactLocation `json:",inline" protobuf:"bytes,2,opt,name=artifactLocation"`
// Target is the stream to listen for artifact notifications
Target Stream `json:"target" protobuf:"bytes,1,opt,name=target"`
}
Either way, this doesn't cause issues when sending signals and events via the Listen() gRPC so it's not a concern right now. However, we can raise an issue to look into this as it relates to the github.com/gogo/protobuf/protoc-gen-gofast or k8s.io/code-generator/cmd/go-to-protobuf tools.
* adding s3 with param example * fixing tests and adding logging to s3 signal
No description provided.