extracting redis streams source #1113
Conversation
Signed-off-by: Julie Vogelman <[email protected]>
Signed-off-by: Julie Vogelman <[email protected]>
Signed-off-by: Julie Vogelman <[email protected]>
Signed-off-by: Julie Vogelman <[email protected]>
Signed-off-by: Julie Vogelman <[email protected]>
Signed-off-by: Julie Vogelman <[email protected]>
Signed-off-by: Julie Vogelman <[email protected]>
Signed-off-by: Julie Vogelman <[email protected]>
|
Remove the redis streams related types? |
Signed-off-by: Julie Vogelman <[email protected]>
Signed-off-by: Julie Vogelman <[email protected]>
good call, done |
Signed-off-by: Julie Vogelman <[email protected]>
pkg/reconciler/pipeline/validate.go
Outdated
| @@ -103,7 +103,7 @@ func ValidatePipeline(pl *dfv1.Pipeline) error { | |||
| if s.Source.UDSource.Container == nil || s.Source.UDSource.Container.Image == "" { | |||
| return fmt.Errorf("invalid user-defined source vertex %q, a customized image is required", k) | |||
| } | |||
| if s.Source.HTTP != nil || s.Source.Kafka != nil || s.Source.Nats != nil || s.Source.RedisStreams != nil || s.Source.Generator != nil { | |||
| if s.Source.HTTP != nil || s.Source.Kafka != nil || s.Source.Nats != nil || s.Source.Generator != nil { | |||
| return fmt.Errorf("invalid user-defined source vertex %q, only one of 'http', 'kafka', 'nats', 'redisStreams', 'generator' and 'udSource' can be specified", k) | |||
There was a problem hiding this comment.
Please also remove redisStreams from the error msg. Maybe we can do a search across the project to ensure we remove all occurrences of "redisstreams", "redis-streams", etc.
There was a problem hiding this comment.
good catch. You know it's weird, I did do:
grep -r -i "redis*stream" .
from the numaflow root but I guess I still missed it :)
There was a problem hiding this comment.
probably because I did it right before I made this particular change to line 106 and then decided I was done :)
Signed-off-by: Julie Vogelman <[email protected]>
pkg/apis/numaflow/v1alpha1/source.go
Outdated
| @@ -36,8 +36,6 @@ type Source struct { | |||
| // +optional | |||
| Nats *NatsSource `json:"nats,omitempty" protobuf:"bytes,4,opt,name=nats"` | |||
| // +optional | |||
| RedisStreams *RedisStreamsSource `json:"redisStreams,omitempty" protobuf:"bytes,5,opt,name=redisStreams"` | |||
| // +optional | |||
| UDTransformer *UDTransformer `json:"transformer,omitempty" protobuf:"bytes,6,opt,name=transformer"` | |||
There was a problem hiding this comment.
Good catch. But just thinking - For Protobuf, in general, you're not supposed to modify existing indices for backwards compatibility purposes. Does that no longer apply for the Protobuf we generate here? (I'm not really sure exactly how Protobuf is being used in this K8S context - but I suppose as long as the "recipient" is also using this same version then we're fine.)
There was a problem hiding this comment.
I don't think any of our users is using protobuf encoding to communicate to k8s, it should be safe to adjust the index.
There was a problem hiding this comment.
The answer to how Protobuf is being used in this K8S context -
https://kubernetes.io/docs/reference/using-api/api-concepts/#alternate-representations-of-resources
There was a problem hiding this comment.
Thanks for sending! In that case, it seems like if something were to use Protobof it would be something which has a running client that can read these or write (create) these, right? I wonder if we would want to have backward compatibility for them, or if they should basically always just adapt to the latest version of the CRD? (I suppose it's a general question for whether to allow changing numbers going forward)
There was a problem hiding this comment.
No one always keeps backward compatibility, projects like k8s have a deprecation process (e.g. mark the fields as deprecated, and remove them in N releases - the numbers are also updated).
There was a problem hiding this comment.
Did you want to follow that process at some point in the future? I know we haven't even released v1.0 yet, so as far as I understand we're not worrying about backward compatibility much right now. I am okay to update the numbers if you prefer.
There was a problem hiding this comment.
We should have similar process later on. Thanks!
Signed-off-by: Julie Vogelman <[email protected]>
Redis Streams Source is now a user-defined source in its own repo