Re: [PR] feat(strimzi): bind to either KafkaTopic name or topicName [camel-k]
squakez merged PR #5281: URL: https://github.com/apache/camel-k/pull/5281 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] feat(strimzi): bind to either KafkaTopic name or topicName [camel-k]
lburgazzoli commented on PR #5281: URL: https://github.com/apache/camel-k/pull/5281#issuecomment-2018277563 ok so I think we should probably follow up with some enhancements in case the operator does not retry. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] feat(strimzi): bind to either KafkaTopic name or topicName [camel-k]
squakez commented on PR #5281: URL: https://github.com/apache/camel-k/pull/5281#issuecomment-2018229684 > I don't recall exactly what is the exact machinery so I wonder what happens when the resource is not found or what would happen if the [status](url) sub resource and/or related [topicName](url) is not set at the time of the binding materialization: does the operator re-try to deploy the resource ? For what I can see in the logic it just fail. I guess this is bubbled up into a Pipe error status. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] feat(strimzi): bind to either KafkaTopic name or topicName [camel-k]
lburgazzoli commented on PR #5281: URL: https://github.com/apache/camel-k/pull/5281#issuecomment-2018223817 I don't recall exactly what is the exact machinery so I wonder what happens when the resource is not found or what would happen if the [status](url) sub resource and/or related [topicName](url) is not set at the time of the binding materialization: does the operator re-try to deploy the resource ? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] feat(strimzi): bind to either KafkaTopic name or topicName [camel-k]
squakez commented on code in PR #5281:
URL: https://github.com/apache/camel-k/pull/5281#discussion_r1537745850
##
addons/strimzi/duck/v1beta2/duck_types.go:
##
@@ -40,6 +40,12 @@ const (
type KafkaTopic struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`
+ StatusKafkaTopicStatus `json:"status,omitempty"`
+}
+
+// KafkaTopicStatus is the duck of a KafkaTopic status.
+type KafkaTopicStatus struct {
+ TopicName string `json:"topicName,omitempty"`
Review Comment:
Should be fixed now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] feat(strimzi): bind to either KafkaTopic name or topicName [camel-k]
squakez commented on code in PR #5281:
URL: https://github.com/apache/camel-k/pull/5281#discussion_r1537708901
##
addons/strimzi/duck/v1beta2/duck_types.go:
##
@@ -40,6 +40,12 @@ const (
type KafkaTopic struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`
+ StatusKafkaTopicStatus `json:"status,omitempty"`
+}
+
+// KafkaTopicStatus is the duck of a KafkaTopic status.
+type KafkaTopicStatus struct {
+ TopicName string `json:"topicName,omitempty"`
Review Comment:
Good point. I'll have a look, thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] feat(strimzi): bind to either KafkaTopic name or topicName [camel-k]
lburgazzoli commented on code in PR #5281:
URL: https://github.com/apache/camel-k/pull/5281#discussion_r1537697837
##
addons/strimzi/duck/v1beta2/duck_types.go:
##
@@ -40,6 +40,12 @@ const (
type KafkaTopic struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`
+ StatusKafkaTopicStatus `json:"status,omitempty"`
+}
+
+// KafkaTopicStatus is the duck of a KafkaTopic status.
+type KafkaTopicStatus struct {
+ TopicName string `json:"topicName,omitempty"`
Review Comment:
Not sure if we need to add a `status` subresource permission
https://github.com/apache/camel-k/blob/main/pkg/resources/config/rbac/descoped/operator-cluster-role-strimzi.yaml
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] feat(strimzi): bind to either KafkaTopic name or topicName [camel-k]
squakez commented on code in PR #5281:
URL: https://github.com/apache/camel-k/pull/5281#discussion_r1537693558
##
addons/strimzi/duck/v1beta2/duck_types.go:
##
@@ -40,6 +40,12 @@ const (
type KafkaTopic struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`
+ Spec KafkaTopicSpec `json:"spec,omitempty"`
+}
+
+// KafkaTopicSpec is the duck of a KafkaTopic spec.
+type KafkaTopicSpec struct {
Review Comment:
Changed to `.status.topicName`. It seems to be the right thing to do, thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] feat(strimzi): bind to either KafkaTopic name or topicName [camel-k]
lburgazzoli commented on code in PR #5281:
URL: https://github.com/apache/camel-k/pull/5281#discussion_r1537689066
##
addons/strimzi/duck/v1beta2/duck_types.go:
##
@@ -40,6 +40,12 @@ const (
type KafkaTopic struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`
+ Spec KafkaTopicSpec `json:"spec,omitempty"`
+}
+
+// KafkaTopicSpec is the duck of a KafkaTopic spec.
+type KafkaTopicSpec struct {
Review Comment:
> I thought the same but I noticed the gen crd was tagged with `noStatus`
(tbh, not sure why), reason why I preferred to be conservative. I can
definitely move to the status if somebody confirm it would be the best thing to
do.
it seems this is for our own processing logic, maybe it is for the RBACs
generation. In the CRD definition I see that the staus is defined
https://github.com/strimzi/strimzi-kafka-operator/blob/main/install/cluster-operator/043-Crd-kafkatopic.yaml#L77-L110.
Maybe we should ask the Strimzi folks about what is the expected behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] feat(strimzi): bind to either KafkaTopic name or topicName [camel-k]
lburgazzoli commented on code in PR #5281:
URL: https://github.com/apache/camel-k/pull/5281#discussion_r1537687133
##
addons/strimzi/duck/v1beta2/duck_types.go:
##
@@ -40,6 +40,12 @@ const (
type KafkaTopic struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`
+ Spec KafkaTopicSpec `json:"spec,omitempty"`
+}
+
+// KafkaTopicSpec is the duck of a KafkaTopic spec.
+type KafkaTopicSpec struct {
Review Comment:
> @lburgazzoli I would say `topicName` as this is the name used by the kafka
broker to create the topic. kafkaTopic name is only the name of the CR that
appears under the api.
>
> Hope that helps ?
I mean the CRD has two "topicName" one in the spec and one in the status.
I think the one in the status is populated only when the topic is actually
created. SO I guess it can reflect either the CR name or the spec's `topicName`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] feat(strimzi): bind to either KafkaTopic name or topicName [camel-k]
aboucham commented on code in PR #5281:
URL: https://github.com/apache/camel-k/pull/5281#discussion_r1537682629
##
addons/strimzi/duck/v1beta2/duck_types.go:
##
@@ -40,6 +40,12 @@ const (
type KafkaTopic struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`
+ Spec KafkaTopicSpec `json:"spec,omitempty"`
+}
+
+// KafkaTopicSpec is the duck of a KafkaTopic spec.
+type KafkaTopicSpec struct {
Review Comment:
@lburgazzoli I would say `topicName` as this is the name used by the kafka
broker to create the topic. kafkaTopic name is only the name of the CR that
appears under the api.
Hope that helps ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] feat(strimzi): bind to either KafkaTopic name or topicName [camel-k]
squakez commented on code in PR #5281:
URL: https://github.com/apache/camel-k/pull/5281#discussion_r1537679273
##
addons/strimzi/duck/v1beta2/duck_types.go:
##
@@ -40,6 +40,12 @@ const (
type KafkaTopic struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`
+ Spec KafkaTopicSpec `json:"spec,omitempty"`
+}
+
+// KafkaTopicSpec is the duck of a KafkaTopic spec.
+type KafkaTopicSpec struct {
Review Comment:
I thought the same but I noticed the gen crd was tagged with `noStatus`
(tbh, not sure why), reason why I preferred to be conservative. I can
definitely move to the status if somebody confirm it would be the best thing to
do.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] feat(strimzi): bind to either KafkaTopic name or topicName [camel-k]
lburgazzoli commented on code in PR #5281:
URL: https://github.com/apache/camel-k/pull/5281#discussion_r1537675545
##
addons/strimzi/duck/v1beta2/duck_types.go:
##
@@ -40,6 +40,12 @@ const (
type KafkaTopic struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`
+ Spec KafkaTopicSpec `json:"spec,omitempty"`
+}
+
+// KafkaTopicSpec is the duck of a KafkaTopic spec.
+type KafkaTopicSpec struct {
Review Comment:
I'm not familiar with the Kafka Topic CRD and the Strimzi Topic operator,
but wonder if we should take the value from the status
https://strimzi.io/docs/operators/latest/configuring.html#type-KafkaTopic-reference
@aboucham any idea ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
