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/30 15:31:15 UTC
[GitHub] [camel-k] essobedo opened a new pull request, #3589: feat(cli): Add tail and tail-lines flags to the log command
essobedo opened a new pull request, #3589:
URL: https://github.com/apache/camel-k/pull/3589
## Motivation
When a pod has a lot of log entries, it can be convenient to be able to get only the last entries of the logs.
## Modifications:
* Add a bool flag `--tail` / `-f` to get the 10 last lines of the logs
* Add an int flag `--tail-lines` / `-l` to get the specified amount of last lines of the logs
**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] tadayosi commented on a diff in pull request #3589: feat(cli): Add tail and tail-lines flags to the log command
Posted by GitBox <gi...@apache.org>.
tadayosi commented on code in PR #3589:
URL: https://github.com/apache/camel-k/pull/3589#discussion_r962551805
##########
pkg/cmd/log.go:
##########
@@ -129,7 +134,13 @@ func (o *logCmdOptions) run(cmd *cobra.Command, args []string) error {
// Found the running integration so step over to scraping its pod log
//
fmt.Fprintln(cmd.OutOrStdout(), "Integration '"+integrationID+"' is now running. Showing log ...")
- if err := k8slog.Print(o.Context, cmd, c, &integration, cmd.OutOrStdout()); err != nil {
+ var tailLines *int64
+ if cmd.Flags().Lookup("tail-lines").Changed {
+ tailLines = &o.TailLines
+ } else if o.Tail {
+ tailLines = &[]int64{10}[0]
+ }
Review Comment:
When `o.TailLines` is explicitly defaulted to `10`, do we really need this `if`? Also, if we do, you should use `pointer.Into64(10)` here:
```
tailLines = &[]int64{10}[0]
```
--
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 #3589: feat(cli): Add tail flag to the log command
Posted by GitBox <gi...@apache.org>.
essobedo commented on PR #3589:
URL: https://github.com/apache/camel-k/pull/3589#issuecomment-1236687716
> By the way since @essobedo is a committer, he could do it by himself.
Yes I know but I'm still new to the project and I'm not a Go expert that's why I'm asking
--
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 #3589: feat(cli): Add tail and tail-lines flags to the log command
Posted by GitBox <gi...@apache.org>.
tadayosi commented on code in PR #3589:
URL: https://github.com/apache/camel-k/pull/3589#discussion_r962595164
##########
pkg/cmd/log.go:
##########
@@ -46,6 +46,9 @@ func newCmdLog(rootCmdOptions *RootCmdOptions) (*cobra.Command, *logCmdOptions)
RunE: options.run,
}
+ cmd.Flags().BoolP("tail", "f", false, "To show the end of the logs")
+ cmd.Flags().Int64P("tail-lines", "l", 10, "The number of lines from the end of the logs to show")
Review Comment:
Or like `kubectl logs` we could possibly drop the original `--tail` and always force passing line numbers with `--tail` without a default value:
https://github.com/kubernetes/kubectl/blob/master/pkg/cmd/logs/logs.go
--
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 a diff in pull request #3589: feat(cli): Add tail and tail-lines flags to the log command
Posted by GitBox <gi...@apache.org>.
essobedo commented on code in PR #3589:
URL: https://github.com/apache/camel-k/pull/3589#discussion_r962575233
##########
pkg/cmd/log.go:
##########
@@ -129,7 +134,13 @@ func (o *logCmdOptions) run(cmd *cobra.Command, args []string) error {
// Found the running integration so step over to scraping its pod log
//
fmt.Fprintln(cmd.OutOrStdout(), "Integration '"+integrationID+"' is now running. Showing log ...")
- if err := k8slog.Print(o.Context, cmd, c, &integration, cmd.OutOrStdout()); err != nil {
+ var tailLines *int64
+ if cmd.Flags().Lookup("tail-lines").Changed {
+ tailLines = &o.TailLines
+ } else if o.Tail {
+ tailLines = &[]int64{10}[0]
+ }
Review Comment:
Good catch, thx. Fixed
--
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 #3589: feat(cli): Add tail and tail-lines flags to the log command
Posted by GitBox <gi...@apache.org>.
essobedo commented on PR #3589:
URL: https://github.com/apache/camel-k/pull/3589#issuecomment-1232578866
The 3 violations raised are not related to these changes https://github.com/apache/camel-k/pull/3589/files#annotation_4463020346
--
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] oscerd commented on pull request #3589: feat(cli): Add tail flag to the log command
Posted by GitBox <gi...@apache.org>.
oscerd commented on PR #3589:
URL: https://github.com/apache/camel-k/pull/3589#issuecomment-1236682960
By the way since @essobedo is a committer, he could do it by himself.
--
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 #3589: feat(cli): Add tail and tail-lines flags to the log command
Posted by GitBox <gi...@apache.org>.
tadayosi commented on code in PR #3589:
URL: https://github.com/apache/camel-k/pull/3589#discussion_r962595164
##########
pkg/cmd/log.go:
##########
@@ -46,6 +46,9 @@ func newCmdLog(rootCmdOptions *RootCmdOptions) (*cobra.Command, *logCmdOptions)
RunE: options.run,
}
+ cmd.Flags().BoolP("tail", "f", false, "To show the end of the logs")
+ cmd.Flags().Int64P("tail-lines", "l", 10, "The number of lines from the end of the logs to show")
Review Comment:
Or like `kubectl logs` we could possibly drop the original `--tail` and always force passing line numbers with `--tail / -l` without a default value:
https://github.com/kubernetes/kubectl/blob/master/pkg/cmd/logs/logs.go
--
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 #3589: feat(cli): Add tail and tail-lines flags to the log command
Posted by GitBox <gi...@apache.org>.
essobedo commented on PR #3589:
URL: https://github.com/apache/camel-k/pull/3589#issuecomment-1233101426
The only failure is on `TestNativeIntegrations/automatic_rollout_deployment_from_fast-jar_to_native_kit` which is a known failing test
--
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 #3589: feat(cli): Add tail and tail-lines flags to the log command
Posted by GitBox <gi...@apache.org>.
essobedo commented on PR #3589:
URL: https://github.com/apache/camel-k/pull/3589#issuecomment-1232543416
@squakez yes, the default behavior remains the same
--
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 #3589: feat(cli): Add tail flag to the log command
Posted by GitBox <gi...@apache.org>.
essobedo commented on PR #3589:
URL: https://github.com/apache/camel-k/pull/3589#issuecomment-1236673046
> I believe we are on hold for merging pull reqs which add a feature and are not related to stabilising e2e just because we've been in the process of releasing 1.10.0 and moving to 1.11.0-SNAPSHOT. When everything is set up, we could resume merging them.
I see it is a specific situation, but what about the process in a normal situation?
--
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 #3589: feat(cli): Add tail and tail-lines flags to the log command
Posted by GitBox <gi...@apache.org>.
tadayosi commented on code in PR #3589:
URL: https://github.com/apache/camel-k/pull/3589#discussion_r962591874
##########
pkg/cmd/log.go:
##########
@@ -46,6 +46,9 @@ func newCmdLog(rootCmdOptions *RootCmdOptions) (*cobra.Command, *logCmdOptions)
RunE: options.run,
}
+ cmd.Flags().BoolP("tail", "f", false, "To show the end of the logs")
+ cmd.Flags().Int64P("tail-lines", "l", 10, "The number of lines from the end of the logs to show")
Review Comment:
I noticed that `-f` shorthand generally means "follow" as in `kubectl logs -f` and our `kamel log` has this behaviour. So `-f` may not be a good shorthand character. `-t` is also not a good shorthand as it's already used in `kamel run` as `--trait`. Maybe we should simply force `--tail`.
--
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 a diff in pull request #3589: feat(cli): Add tail and tail-lines flags to the log command
Posted by GitBox <gi...@apache.org>.
essobedo commented on code in PR #3589:
URL: https://github.com/apache/camel-k/pull/3589#discussion_r962597875
##########
pkg/cmd/log.go:
##########
@@ -46,6 +46,9 @@ func newCmdLog(rootCmdOptions *RootCmdOptions) (*cobra.Command, *logCmdOptions)
RunE: options.run,
}
+ cmd.Flags().BoolP("tail", "f", false, "To show the end of the logs")
+ cmd.Flags().Int64P("tail-lines", "l", 10, "The number of lines from the end of the logs to show")
Review Comment:
Good idea, let's use the same behavior as `kubectl logs`:
```
--tail=-1:
Lines of recent log file to display. Defaults to -1 with no selector, showing all log lines otherwise 10, if a
selector is provided.
```
--
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 #3589: feat(cli): Add tail and tail-lines flags to the log command
Posted by GitBox <gi...@apache.org>.
tadayosi commented on PR #3589:
URL: https://github.com/apache/camel-k/pull/3589#issuecomment-1236665476
I believe we are on hold for merging pull reqs which add a feature and are not related to stabilising e2e just because we've been in the process of releasing 1.10.0 and moving to 1.11.0-SNAPSHOT. When everything is set up, we could resume merging them.
--
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 #3589: feat(cli): Add tail flag to the log command
Posted by GitBox <gi...@apache.org>.
tadayosi commented on PR #3589:
URL: https://github.com/apache/camel-k/pull/3589#issuecomment-1236681904
> I see it is a specific situation, but what about the process in a normal situation?
@essobedo Basically, once a PR has enough approvals and all CI checks have passed anyone with "Member" right to this repo could merge it. Practically, @squakez, I, and @oscerd would take care of them for most cases these days.
--
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 #3589: feat(cli): Add tail and tail-lines flags to the log command
Posted by GitBox <gi...@apache.org>.
essobedo commented on PR #3589:
URL: https://github.com/apache/camel-k/pull/3589#issuecomment-1236648937
BTW how do you proceed in camel-k in terms of PR validation? I mean: How many PR approvals are needed? Who merges 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] squakez commented on pull request #3589: feat(cli): Add tail flag to the log command
Posted by GitBox <gi...@apache.org>.
squakez commented on PR #3589:
URL: https://github.com/apache/camel-k/pull/3589#issuecomment-1236755632
> > By the way since @essobedo is a committer, he could do it by himself.
>
> Yes I know but I'm still new to the project and I'm not a Go expert that's why I'm asking
No worries. If the PR was approved by some committer and checks passed, then, you can merge yourself. I usually wait 24/48h more to see any other review. Also, I prefer to apply "rebase and merge" strategy in order to have a linear story of the commits on the main branch.
--
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 a diff in pull request #3589: feat(cli): Add tail and tail-lines flags to the log command
Posted by GitBox <gi...@apache.org>.
essobedo commented on code in PR #3589:
URL: https://github.com/apache/camel-k/pull/3589#discussion_r962593898
##########
pkg/cmd/log.go:
##########
@@ -46,6 +46,9 @@ func newCmdLog(rootCmdOptions *RootCmdOptions) (*cobra.Command, *logCmdOptions)
RunE: options.run,
}
+ cmd.Flags().BoolP("tail", "f", false, "To show the end of the logs")
+ cmd.Flags().Int64P("tail-lines", "l", 10, "The number of lines from the end of the logs to show")
Review Comment:
I agree, let's do that
--
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 #3589: feat(cli): Add tail flag to the log command
Posted by GitBox <gi...@apache.org>.
tadayosi commented on PR #3589:
URL: https://github.com/apache/camel-k/pull/3589#issuecomment-1238897405
Looks all good. Let's merge.
--
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 #3589: feat(cli): Add tail flag to the log command
Posted by GitBox <gi...@apache.org>.
tadayosi merged PR #3589:
URL: https://github.com/apache/camel-k/pull/3589
--
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