You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2021/08/16 17:29:06 UTC
[GitHub] [camel-k] squakez opened a new pull request #2575: fix(cmd/bind): show expected kameletbinding
squakez opened a new pull request #2575:
URL: https://github.com/apache/camel-k/pull/2575
Closes #2565
<!-- Description -->
<!--
Enter your extended release note in the below block. If the PR requires
additional action from users switching to the new release, include the string
"action required". If no release note is required, write "NONE".
You can (optionally) mark this PR with labels "kind/bug" or "kind/feature" to make sure
the text is added to the right section of the release notes.
-->
**Release Note**
```release-note
NONE
```
--
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-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on a change in pull request #2575: fix(cmd/bind): show expected kameletbinding
Posted by GitBox <gi...@apache.org>.
squakez commented on a change in pull request #2575:
URL: https://github.com/apache/camel-k/pull/2575#discussion_r691913346
##########
File path: pkg/cmd/bind.go
##########
@@ -206,6 +193,34 @@ func (o *bindCmdOptions) run(args []string) error {
return nil
}
+func showOutput(cmd *cobra.Command, binding *v1alpha1.KameletBinding, outputFormat string) error {
Review comment:
I've tried to follow the approach you proposed. In order to make it works, we need to use a newer version of Golang (1.16) as those libraries seems to require it. [From a separate discussion](https://github.com/apache/camel-k/issues/2579) I understood we're not yet ready for it. Should we progress and then apply your suggestion once we move to 1.16?
--
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-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on a change in pull request #2575: fix(cmd/bind): show expected kameletbinding
Posted by GitBox <gi...@apache.org>.
squakez commented on a change in pull request #2575:
URL: https://github.com/apache/camel-k/pull/2575#discussion_r692100442
##########
File path: pkg/cmd/bind.go
##########
@@ -206,6 +193,34 @@ func (o *bindCmdOptions) run(args []string) error {
return nil
}
+func showOutput(cmd *cobra.Command, binding *v1alpha1.KameletBinding, outputFormat string) error {
Review comment:
Using `k8s.io/cli-runtime v0.20.2` seems okey with go 1.5. I'm trying to see how I can use it accordingly now, 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-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on a change in pull request #2575: fix(cmd/bind): show expected kameletbinding
Posted by GitBox <gi...@apache.org>.
squakez commented on a change in pull request #2575:
URL: https://github.com/apache/camel-k/pull/2575#discussion_r692179322
##########
File path: pkg/cmd/bind.go
##########
@@ -206,6 +189,14 @@ func (o *bindCmdOptions) run(args []string) error {
return nil
}
+func showOutput(cmd *cobra.Command, binding *v1alpha1.KameletBinding, outputFormat string) error {
+ printer := printers.NewTypeSetter(scheme.Scheme)
Review comment:
Yes, I thought it was not available in the `cmd`, but I manage to find it moving the output logic after creating the `client`. Fixed now, 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-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on a change in pull request #2575: fix(cmd/bind): show expected kameletbinding
Posted by GitBox <gi...@apache.org>.
squakez commented on a change in pull request #2575:
URL: https://github.com/apache/camel-k/pull/2575#discussion_r692130298
##########
File path: pkg/cmd/bind.go
##########
@@ -206,6 +193,34 @@ func (o *bindCmdOptions) run(args []string) error {
return nil
}
+func showOutput(cmd *cobra.Command, binding *v1alpha1.KameletBinding, outputFormat string) error {
Review comment:
I've just committed the changes. Thanks for the suggestion, it looks very neat now and hopefully we can reuse this logic for any other runtime object! Please, have a look if it fits with what you had in mind.
--
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-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti merged pull request #2575: fix(cmd/bind): show expected kameletbinding
Posted by GitBox <gi...@apache.org>.
astefanutti merged pull request #2575:
URL: https://github.com/apache/camel-k/pull/2575
--
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-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] nicolaferraro commented on a change in pull request #2575: fix(cmd/bind): show expected kameletbinding
Posted by GitBox <gi...@apache.org>.
nicolaferraro commented on a change in pull request #2575:
URL: https://github.com/apache/camel-k/pull/2575#discussion_r692153422
##########
File path: pkg/apis/camel/v1alpha1/kamelet_binding_types.go
##########
@@ -134,3 +136,21 @@ type KameletBindingList struct {
metav1.ListMeta `json:"metadata,omitempty"`
Items []KameletBinding `json:"items"`
}
+
+// ObjectKinds return the GKV of the KameletBinding spec
+func (kb *KameletBinding) ObjectKinds(runtime.Object) ([]schema.GroupVersionKind, bool, error) {
+ return []schema.GroupVersionKind{
+ {
+ Group: SchemeGroupVersion.Group,
+ Version: SchemeGroupVersion.Version,
+ Kind: KameletBindingKind,
+ },
+ }, true, nil
+}
+
+// Recognizes returns true if it match the expected GKV
+func (kb *KameletBinding) Recognizes(schema schema.GroupVersionKind) bool {
+ return schema.Group == SchemeGroupVersion.Group &&
+ schema.Kind == KameletBindingKind &&
+ schema.Version == SchemeGroupVersion.Version
+}
Review comment:
@squakez why are they needed? (still trying to understand why #2582 CI fails)
--
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-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on a change in pull request #2575: fix(cmd/bind): show expected kameletbinding
Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #2575:
URL: https://github.com/apache/camel-k/pull/2575#discussion_r692172862
##########
File path: pkg/cmd/bind.go
##########
@@ -206,6 +189,14 @@ func (o *bindCmdOptions) run(args []string) error {
return nil
}
+func showOutput(cmd *cobra.Command, binding *v1alpha1.KameletBinding, outputFormat string) error {
+ printer := printers.NewTypeSetter(scheme.Scheme)
Review comment:
That's the default scheme. My understanding is that the scheme initialised for the command client should be passed.
--
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-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on a change in pull request #2575: fix(cmd/bind): show expected kameletbinding
Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #2575:
URL: https://github.com/apache/camel-k/pull/2575#discussion_r691048718
##########
File path: pkg/cmd/bind.go
##########
@@ -206,6 +193,34 @@ func (o *bindCmdOptions) run(args []string) error {
return nil
}
+func showOutput(cmd *cobra.Command, binding *v1alpha1.KameletBinding, outputFormat string) error {
Review comment:
The _schema_ is the concept that is responsible to map GKV and types, so it may be used here, rather than doing it ourselves. It implements the `runtime.ObjectTyper` interface, that can be used to add the GKV information. Also it may be possible to rely on the `cli-runtime` package, which is used by `kubectl`:
https://github.com/kubernetes/cli-runtime/blob/c2f4736118ac4e1fd42fdd347954747433ca4fb7/pkg/printers/typesetter.go#L35
--
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-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on a change in pull request #2575: fix(cmd/bind): show expected kameletbinding
Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #2575:
URL: https://github.com/apache/camel-k/pull/2575#discussion_r692155996
##########
File path: pkg/apis/camel/v1alpha1/kamelet_binding_types.go
##########
@@ -134,3 +136,21 @@ type KameletBindingList struct {
metav1.ListMeta `json:"metadata,omitempty"`
Items []KameletBinding `json:"items"`
}
+
+// ObjectKinds return the GKV of the KameletBinding spec
+func (kb *KameletBinding) ObjectKinds(runtime.Object) ([]schema.GroupVersionKind, bool, error) {
+ return []schema.GroupVersionKind{
+ {
+ Group: SchemeGroupVersion.Group,
+ Version: SchemeGroupVersion.Version,
+ Kind: KameletBindingKind,
+ },
+ }, true, nil
+}
+
+// Recognizes returns true if it match the expected GKV
+func (kb *KameletBinding) Recognizes(schema schema.GroupVersionKind) bool {
+ return schema.Group == SchemeGroupVersion.Group &&
+ schema.Kind == KameletBindingKind &&
+ schema.Version == SchemeGroupVersion.Version
+}
Review comment:
@nicolaferraro that's the implementation for `runtime.ObjectTyper`, but that's supposed to be implemented by the scheme, into which the `KameletBinding` type should be registered.
--
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-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on a change in pull request #2575: fix(cmd/bind): show expected kameletbinding
Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #2575:
URL: https://github.com/apache/camel-k/pull/2575#discussion_r692141400
##########
File path: pkg/cmd/bind.go
##########
@@ -206,6 +193,34 @@ func (o *bindCmdOptions) run(args []string) error {
return nil
}
+func showOutput(cmd *cobra.Command, binding *v1alpha1.KameletBinding, outputFormat string) error {
Review comment:
Great, my understanding is that the scheme abstraction is meant to be used so that it works with any runtime object that's registered into the scheme:
https://github.com/kubernetes/kubectl/blob/4defba0cec1f594eb410c69bff05b51cddfba8ff/pkg/cmd/get/get.go#L266
Every online command should already have the scheme initialised correctly, and accessible from `client.GetScheme()`.
--
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-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on a change in pull request #2575: fix(cmd/bind): show expected kameletbinding
Posted by GitBox <gi...@apache.org>.
squakez commented on a change in pull request #2575:
URL: https://github.com/apache/camel-k/pull/2575#discussion_r692167282
##########
File path: pkg/apis/camel/v1alpha1/kamelet_binding_types.go
##########
@@ -134,3 +136,21 @@ type KameletBindingList struct {
metav1.ListMeta `json:"metadata,omitempty"`
Items []KameletBinding `json:"items"`
}
+
+// ObjectKinds return the GKV of the KameletBinding spec
+func (kb *KameletBinding) ObjectKinds(runtime.Object) ([]schema.GroupVersionKind, bool, error) {
+ return []schema.GroupVersionKind{
+ {
+ Group: SchemeGroupVersion.Group,
+ Version: SchemeGroupVersion.Version,
+ Kind: KameletBindingKind,
+ },
+ }, true, nil
+}
+
+// Recognizes returns true if it match the expected GKV
+func (kb *KameletBinding) Recognizes(schema schema.GroupVersionKind) bool {
+ return schema.Group == SchemeGroupVersion.Group &&
+ schema.Kind == KameletBindingKind &&
+ schema.Version == SchemeGroupVersion.Version
+}
Review comment:
Yeah, I had to implement those methods as I was using `printer := printers.NewTypeSetter(binding)` where binding was a `*v1alpha1.KameletBinding`. According to the other comment @astefanutti did previously, this would be no longer needed if I use `scheme.Scheme`. Would it be the right way to proceed?
--
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-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on a change in pull request #2575: fix(cmd/bind): show expected kameletbinding
Posted by GitBox <gi...@apache.org>.
squakez commented on a change in pull request #2575:
URL: https://github.com/apache/camel-k/pull/2575#discussion_r692171491
##########
File path: pkg/apis/camel/v1alpha1/kamelet_binding_types.go
##########
@@ -134,3 +136,21 @@ type KameletBindingList struct {
metav1.ListMeta `json:"metadata,omitempty"`
Items []KameletBinding `json:"items"`
}
+
+// ObjectKinds return the GKV of the KameletBinding spec
+func (kb *KameletBinding) ObjectKinds(runtime.Object) ([]schema.GroupVersionKind, bool, error) {
+ return []schema.GroupVersionKind{
+ {
+ Group: SchemeGroupVersion.Group,
+ Version: SchemeGroupVersion.Version,
+ Kind: KameletBindingKind,
+ },
+ }, true, nil
+}
+
+// Recognizes returns true if it match the expected GKV
+func (kb *KameletBinding) Recognizes(schema schema.GroupVersionKind) bool {
+ return schema.Group == SchemeGroupVersion.Group &&
+ schema.Kind == KameletBindingKind &&
+ schema.Version == SchemeGroupVersion.Version
+}
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-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on a change in pull request #2575: fix(cmd/bind): show expected kameletbinding
Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #2575:
URL: https://github.com/apache/camel-k/pull/2575#discussion_r691945006
##########
File path: pkg/cmd/bind.go
##########
@@ -206,6 +193,34 @@ func (o *bindCmdOptions) run(args []string) error {
return nil
}
+func showOutput(cmd *cobra.Command, binding *v1alpha1.KameletBinding, outputFormat string) error {
Review comment:
Doesn't it work by depending on the 1.20 version: https://github.com/kubernetes/cli-runtime/blob/release-1.20/go.mod?
Otherwise, following the Golang version from k8s is rather a rule of thumb, and more a minorant. We can try upgrading to 1.16 if needs be.
--
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-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on a change in pull request #2575: fix(cmd/bind): show expected kameletbinding
Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #2575:
URL: https://github.com/apache/camel-k/pull/2575#discussion_r691048718
##########
File path: pkg/cmd/bind.go
##########
@@ -206,6 +193,34 @@ func (o *bindCmdOptions) run(args []string) error {
return nil
}
+func showOutput(cmd *cobra.Command, binding *v1alpha1.KameletBinding, outputFormat string) error {
Review comment:
The _schema_ is the concept that is responsible to map GKV and types, so it may be used here, rather than doing it ourselves. It implements the `runtime.ObjectTyper` interface, that can be used to add the GKV information. Also it may be possible to rely on the `cli-runtime` package, which is used by `kubectl`:
https://github.com/kubernetes/cli-runtime/blob/c2f4736118ac4e1fd42fdd347954747433ca4fb7/pkg/printers/typesetter.go#L35
--
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-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org