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 2022/08/19 09:54:28 UTC
[GitHub] [camel-k] essobedo opened a new pull request, #3556: feat(cli): Force arguments for the rebuild command
essobedo opened a new pull request, #3556:
URL: https://github.com/apache/camel-k/pull/3556
fixes #3445
## Motivation
Allowing to accept no arguments to rebuild all the integrations is a bit risky so a new flag for this purpose must be explicitly provided by the end user.
## Modifications:
* Add the flag `--all` to the rebuild command to rebuild all integrations
* Sanitize the name of the integration to align the code with the delete command
**Release Note**
```The rebuild command no more support empty arguments, either the flag `--all` must be set to rebuild all integrations or the list of integration names to rebuild needs to be provided
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] tadayosi merged pull request #3556: feat(cli): Force arguments for the rebuild command
Posted by GitBox <gi...@apache.org>.
tadayosi merged PR #3556:
URL: https://github.com/apache/camel-k/pull/3556
--
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 diff in pull request #3556: feat(cli): Force arguments for the rebuild command
Posted by GitBox <gi...@apache.org>.
squakez commented on code in PR #3556:
URL: https://github.com/apache/camel-k/pull/3556#discussion_r951100176
##########
pkg/cmd/rebuild.go:
##########
@@ -84,9 +108,10 @@ func (o *rebuildCmdOptions) listAllIntegrations(c client.Client) ([]v1.Integrati
func (o *rebuildCmdOptions) getIntegrations(c client.Client, names []string) ([]v1.Integration, error) {
ints := make([]v1.Integration, 0, len(names))
for _, n := range names {
- it := v1.NewIntegration(o.Namespace, n)
+ name := kubernetes.SanitizeName(n)
Review Comment:
Why do we need to sanitize the name? Isn't this value already the one passed by the user?
--
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] essobedo commented on pull request #3556: feat(cli): Force arguments for the rebuild command
Posted by GitBox <gi...@apache.org>.
essobedo commented on PR #3556:
URL: https://github.com/apache/camel-k/pull/3556#issuecomment-1222127066
Ok, No problem, I will do that next week then thx
--
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] essobedo commented on pull request #3556: feat(cli): Force arguments for the rebuild command
Posted by GitBox <gi...@apache.org>.
essobedo commented on PR #3556:
URL: https://github.com/apache/camel-k/pull/3556#issuecomment-1222072199
@tadayosi @squakez Thx for your feedback, actually the remarks that you both raised are about the code that I reused from the `delete` command for the sake of uniformity since the need is about the same. It is up to you but if I change this, maybe I should also align the code of the delete command, don't you agree?
--
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] tadayosi commented on a diff in pull request #3556: feat(cli): Force arguments for the rebuild command
Posted by GitBox <gi...@apache.org>.
tadayosi commented on code in PR #3556:
URL: https://github.com/apache/camel-k/pull/3556#discussion_r950998567
##########
pkg/cmd/rebuild.go:
##########
@@ -27,25 +27,49 @@ import (
v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
"github.com/apache/camel-k/pkg/client"
+ "github.com/apache/camel-k/pkg/util/kubernetes"
)
func newCmdRebuild(rootCmdOptions *RootCmdOptions) (*cobra.Command, *rebuildCmdOptions) {
options := rebuildCmdOptions{
RootCmdOptions: rootCmdOptions,
}
cmd := cobra.Command{
- Use: "rebuild [integration]",
+ Use: "rebuild [integration1] [integration2] ...",
Short: "Clear the state of integrations to rebuild them",
Long: `Clear the state of one or more integrations causing a rebuild.`,
PreRunE: decode(&options),
- RunE: options.rebuild,
+ RunE: func(cmd *cobra.Command, args []string) error {
+ if err := options.validate(args); err != nil {
+ return err
+ }
+ if err := options.rebuild(cmd, args); err != nil {
+ fmt.Fprintln(cmd.ErrOrStderr(), err.Error())
+ }
+
+ return nil
+ },
}
+ cmd.Flags().Bool("all", false, "Rebuild all integrations")
+
return &cmd, &options
}
type rebuildCmdOptions struct {
*RootCmdOptions
+ RebuildAll bool `mapstructure:"all"`
+}
+
+func (o *rebuildCmdOptions) validate(args []string) error {
+ if o.RebuildAll && len(args) > 0 {
+ return errors.New("invalid combination: both all flag and named integrations are set")
+ }
+ if !o.RebuildAll && len(args) == 0 {
+ return errors.New("invalid combination: neither all flag nor named integrations are set")
Review Comment:
The error message is not intuitive a little bit, as the user would've just run `kamel rebuild` without any combination of flags & args. You can just suggest either specifying integration name(s) or --all flag.
##########
pkg/cmd/rebuild.go:
##########
@@ -27,25 +27,49 @@ import (
v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
"github.com/apache/camel-k/pkg/client"
+ "github.com/apache/camel-k/pkg/util/kubernetes"
)
func newCmdRebuild(rootCmdOptions *RootCmdOptions) (*cobra.Command, *rebuildCmdOptions) {
options := rebuildCmdOptions{
RootCmdOptions: rootCmdOptions,
}
cmd := cobra.Command{
- Use: "rebuild [integration]",
+ Use: "rebuild [integration1] [integration2] ...",
Short: "Clear the state of integrations to rebuild them",
Long: `Clear the state of one or more integrations causing a rebuild.`,
PreRunE: decode(&options),
- RunE: options.rebuild,
+ RunE: func(cmd *cobra.Command, args []string) error {
+ if err := options.validate(args); err != nil {
+ return err
+ }
+ if err := options.rebuild(cmd, args); err != nil {
Review Comment:
You can just return the method result directly as there's no cleanup afterwards. And no need for `return nil` at the bottom of the func.
--
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 pull request #3556: feat(cli): Force arguments for the rebuild command
Posted by GitBox <gi...@apache.org>.
squakez commented on PR #3556:
URL: https://github.com/apache/camel-k/pull/3556#issuecomment-1222125281
We can take the opportunity and do the necessary cleaning on the other commands as well.
--
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