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/04/12 10:37:54 UTC
[GitHub] [camel-k] johnpoth opened a new pull request, #3193: Add verbose flag for the run command
johnpoth opened a new pull request, #3193:
URL: https://github.com/apache/camel-k/pull/3193
<!-- Description -->
Thanks !
<!--
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] johnpoth commented on a diff in pull request #3193: Add verbose flag for the run command
Posted by GitBox <gi...@apache.org>.
johnpoth commented on code in PR #3193:
URL: https://github.com/apache/camel-k/pull/3193#discussion_r863542752
##########
pkg/cmd/run.go:
##########
@@ -1032,15 +1032,19 @@ func writeChecksumToFile(filepath string, hash hash.Hash) error {
return err
}
-func getSpectrumOptions(platform *v1.IntegrationPlatform, cmd *cobra.Command) spectrum.Options {
+func (o *runCmdOptions) getSpectrumOptions(platform *v1.IntegrationPlatform, cmd *cobra.Command) spectrum.Options {
insecure := platform.Spec.Build.Registry.Insecure
+ var stdout io.Writer
+ if o.Verbose {
+ stdout = cmd.OutOrStdout()
+ }
Review Comment:
Hi @squakez if it's false (default) then *stdout* stays nil and nothing will be sent to the console
--
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 pull request #3193: Add verbose flag for the run command
Posted by GitBox <gi...@apache.org>.
tadayosi commented on PR #3193:
URL: https://github.com/apache/camel-k/pull/3193#issuecomment-1097582512
> Additionally I'd wrap a function, ie, printOut(verbose bool) where you have the if condition, and call directly this function within the rest of the code. We will have a more readable code.
That sounds a nice idea. I'd push it further and make wrapped printing functions as methods for `rootCmdOptions` so that all child commands can exploit them. Maybe we could have `o.printOut()`, `o.printErr()`, `o.printVerboseOut()`, and `o.printVerboseErr()`.
--
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] johnpoth commented on pull request #3193: Add verbose flag for the run command
Posted by GitBox <gi...@apache.org>.
johnpoth commented on PR #3193:
URL: https://github.com/apache/camel-k/pull/3193#issuecomment-1113175852
maybe worth switching to using *klog* like we do in the operator
--
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 #3193: Add verbose flag for the run command
Posted by GitBox <gi...@apache.org>.
tadayosi commented on code in PR #3193:
URL: https://github.com/apache/camel-k/pull/3193#discussion_r850059517
##########
pkg/cmd/run.go:
##########
@@ -117,6 +117,7 @@ func newCmdRun(rootCmdOptions *RootCmdOptions) (*cobra.Command, *runCmdOptions)
cmd.Flags().String("pod-template", "", "The path of the YAML file containing a PodSpec template to be used for the Integration pods")
cmd.Flags().Bool("save", false, "Save the run parameters into the default kamel configuration file (kamel-config.yaml)")
+ cmd.Flags().Bool("verbose", false, "Verbose logging")
Review Comment:
Ah I see, we already have `--volume`... Maybe then we can use `-V`?
It's ok to leave it without a shorthand flag if we don't find a good one.
--
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] johnpoth commented on pull request #3193: Add verbose flag for the run command
Posted by GitBox <gi...@apache.org>.
johnpoth commented on PR #3193:
URL: https://github.com/apache/camel-k/pull/3193#issuecomment-1114633157
Sounds good ! Applied the changes, 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] tadayosi commented on a diff in pull request #3193: Add verbose flag for the run command
Posted by GitBox <gi...@apache.org>.
tadayosi commented on code in PR #3193:
URL: https://github.com/apache/camel-k/pull/3193#discussion_r849089290
##########
pkg/cmd/run.go:
##########
@@ -782,15 +788,15 @@ func resolvePodTemplate(ctx context.Context, cmd *cobra.Command, templateSrc str
return err
}
-func uploadFileOrDirectory(platform *v1.IntegrationPlatform, item string, integrationName string, cmd *cobra.Command, integration *v1.Integration) error {
+func uploadFileOrDirectory(platform *v1.IntegrationPlatform, item string, integrationName string, cmd *cobra.Command, integration *v1.Integration, verbose bool) error {
Review Comment:
Maybe we can change it into method of `runCmdOptions` as the args now looks a bit too long.
##########
pkg/cmd/run.go:
##########
@@ -945,14 +955,16 @@ func extractGav(src *zip.File, localPath string) (maven.Dependency, bool) {
}, true
}
-func uploadAsMavenArtifact(dependency maven.Dependency, path string, platform *v1.IntegrationPlatform, ns string, options spectrum.Options) error {
+func uploadAsMavenArtifact(dependency maven.Dependency, path string, platform *v1.IntegrationPlatform, ns string, options spectrum.Options, verbose bool) error {
Review Comment:
Maybe we can change it into method of `runCmdOptions` as the args now looks a bit too long.
##########
pkg/cmd/run.go:
##########
@@ -1028,15 +1040,19 @@ func writeChecksumToFile(filepath string, hash hash.Hash) error {
return err
}
-func getSpectrumOptions(platform *v1.IntegrationPlatform, cmd *cobra.Command) spectrum.Options {
+func getSpectrumOptions(platform *v1.IntegrationPlatform, cmd *cobra.Command, verbose bool) spectrum.Options {
Review Comment:
Maybe we can change it into method of `runCmdOptions` as the args now looks a bit too long.
##########
pkg/cmd/run.go:
##########
@@ -117,6 +117,7 @@ func newCmdRun(rootCmdOptions *RootCmdOptions) (*cobra.Command, *runCmdOptions)
cmd.Flags().String("pod-template", "", "The path of the YAML file containing a PodSpec template to be used for the Integration pods")
cmd.Flags().Bool("save", false, "Save the run parameters into the default kamel configuration file (kamel-config.yaml)")
+ cmd.Flags().Bool("verbose", false, "Verbose logging")
Review Comment:
Is there no shorthand name for the flag? :smiley:
It looks like the `verbose` flag can be global, but maybe we can work on it later?
##########
pkg/cmd/run.go:
##########
@@ -850,7 +860,7 @@ func getMountPath(targetPath string, dirName string, path string) (string, error
}
// nolint:errcheck
-func uploadPomFromJar(gav maven.Dependency, path string, platform *v1.IntegrationPlatform, ns string, options spectrum.Options) maven.Dependency {
+func uploadPomFromJar(gav maven.Dependency, path string, platform *v1.IntegrationPlatform, ns string, options spectrum.Options, verbose bool) maven.Dependency {
Review Comment:
Maybe we can change it into method of `runCmdOptions` as the args now looks a bit too long.
--
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 #3193: Add verbose flag for the run command
Posted by GitBox <gi...@apache.org>.
squakez commented on code in PR #3193:
URL: https://github.com/apache/camel-k/pull/3193#discussion_r863534964
##########
pkg/cmd/run.go:
##########
@@ -1032,15 +1032,19 @@ func writeChecksumToFile(filepath string, hash hash.Hash) error {
return err
}
-func getSpectrumOptions(platform *v1.IntegrationPlatform, cmd *cobra.Command) spectrum.Options {
+func (o *runCmdOptions) getSpectrumOptions(platform *v1.IntegrationPlatform, cmd *cobra.Command) spectrum.Options {
insecure := platform.Spec.Build.Registry.Insecure
+ var stdout io.Writer
+ if o.Verbose {
+ stdout = cmd.OutOrStdout()
+ }
Review Comment:
What happen if `o.Verbose==false`? is it okey if we leave the `nil` or is it going to break in the Spectrum struct?
--
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 #3193: Add verbose flag for the run command
Posted by GitBox <gi...@apache.org>.
squakez commented on PR #3193:
URL: https://github.com/apache/camel-k/pull/3193#issuecomment-1113302673
> maybe worth switching to using _klog_ like we do in the operator
Sure. Alternatively we can finalize this and make a follow up issue to do it.
--
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] johnpoth commented on a diff in pull request #3193: Add verbose flag for the run command
Posted by GitBox <gi...@apache.org>.
johnpoth commented on code in PR #3193:
URL: https://github.com/apache/camel-k/pull/3193#discussion_r849323245
##########
pkg/cmd/run.go:
##########
@@ -117,6 +117,7 @@ func newCmdRun(rootCmdOptions *RootCmdOptions) (*cobra.Command, *runCmdOptions)
cmd.Flags().String("pod-template", "", "The path of the YAML file containing a PodSpec template to be used for the Integration pods")
cmd.Flags().Bool("save", false, "Save the run parameters into the default kamel configuration file (kamel-config.yaml)")
+ cmd.Flags().Bool("verbose", false, "Verbose logging")
Review Comment:
Ah yeah b/c it was already taken by the volume flag ... wasn't sure what to do in that case :) vb ?
--
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] johnpoth commented on pull request #3193: Add verbose flag for the run command
Posted by GitBox <gi...@apache.org>.
johnpoth commented on PR #3193:
URL: https://github.com/apache/camel-k/pull/3193#issuecomment-1115843129
Thanks @squakez @tadayosi for the review! merging ...
--
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] johnpoth merged pull request #3193: Add verbose flag for the run command
Posted by GitBox <gi...@apache.org>.
johnpoth merged PR #3193:
URL: https://github.com/apache/camel-k/pull/3193
--
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