You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2022/11/06 14:15:38 UTC
[GitHub] [skywalking-infra-e2e] fgksgf opened a new pull request, #92: Add `batchOutput` config to reduce outputs
fgksgf opened a new pull request, #92:
URL: https://github.com/apache/skywalking-infra-e2e/pull/92
In some scenarios like CI, e2e will produce excessive output because it can not rewind back to the first character of the line. Therefore this PR add a `batchOutput` config to solve the issue.
- when `batchOuput` is enabled (default):
- when `batchOuput` is disabled:
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-infra-e2e] kezhenxu94 commented on a diff in pull request #92: Add `batchOutput` config to reduce outputs
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on code in PR #92:
URL: https://github.com/apache/skywalking-infra-e2e/pull/92#discussion_r1014839079
##########
docs/en/setup/Configuration-File.md:
##########
@@ -161,6 +161,7 @@ verify:
interval: 10s # the interval between two attempts, e.g. 10s, 1m.
fail-fast: true # when a case fails, whether to stop verifying other cases. This property defaults to true.
concurrency: false # whether to verify cases concurrently. This property defaults to false.
+ batchOutput: true # whether to output the result of verify cases in batch. This property defaults to true.
Review Comment:
Hi, I'd move this to command line options `--batch/-B`, not in configuration file, otherwise we have to modify the configuration file when running locally and revert back when submit the codes into codebase.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-infra-e2e] kezhenxu94 commented on a diff in pull request #92: Add `batchOutput` config to reduce outputs
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on code in PR #92:
URL: https://github.com/apache/skywalking-infra-e2e/pull/92#discussion_r1014839079
##########
docs/en/setup/Configuration-File.md:
##########
@@ -161,6 +161,7 @@ verify:
interval: 10s # the interval between two attempts, e.g. 10s, 1m.
fail-fast: true # when a case fails, whether to stop verifying other cases. This property defaults to true.
concurrency: false # whether to verify cases concurrently. This property defaults to false.
+ batchOutput: true # whether to output the result of verify cases in batch. This property defaults to true.
Review Comment:
Hi, I'd move this to command line options `--batch/-B`, not in configuration file, otherwise we have to modify the configuration file when running locally and revert back when submit the codes into codebase, then this option becomes less useful as it should 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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-infra-e2e] wu-sheng merged pull request #92: Add `batchOutput` config to reduce outputs
Posted by GitBox <gi...@apache.org>.
wu-sheng merged PR #92:
URL: https://github.com/apache/skywalking-infra-e2e/pull/92
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-infra-e2e] fgksgf commented on a diff in pull request #92: Add `batchOutput` config to reduce outputs
Posted by GitBox <gi...@apache.org>.
fgksgf commented on code in PR #92:
URL: https://github.com/apache/skywalking-infra-e2e/pull/92#discussion_r1021507832
##########
commands/root.go:
##########
@@ -94,6 +93,8 @@ func Execute() error {
Root.PersistentFlags().StringVarP(&util.WorkDir, "work-dir", "w", "~/.skywalking-infra-e2e", "the working directory for skywalking-infra-e2e")
Root.PersistentFlags().StringVarP(&util.LogDir, "log-dir", "l", "~/.skywalking-infra-e2e/logs", "the container logs directory for environment")
Root.PersistentFlags().StringVarP(&util.CfgFile, "config", "c", constant.E2EDefaultFile, "the config file")
+ Root.PersistentFlags().BoolVarP(&util.BatchOutput, "batchOutput", "b", true,
Review Comment:
Make sense to me.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-infra-e2e] kezhenxu94 commented on a diff in pull request #92: Add `batchOutput` config to reduce outputs
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on code in PR #92:
URL: https://github.com/apache/skywalking-infra-e2e/pull/92#discussion_r1020923518
##########
commands/root.go:
##########
@@ -94,6 +93,8 @@ func Execute() error {
Root.PersistentFlags().StringVarP(&util.WorkDir, "work-dir", "w", "~/.skywalking-infra-e2e", "the working directory for skywalking-infra-e2e")
Root.PersistentFlags().StringVarP(&util.LogDir, "log-dir", "l", "~/.skywalking-infra-e2e/logs", "the container logs directory for environment")
Root.PersistentFlags().StringVarP(&util.CfgFile, "config", "c", constant.E2EDefaultFile, "the config file")
+ Root.PersistentFlags().BoolVarP(&util.BatchOutput, "batchOutput", "b", true,
Review Comment:
```suggestion
Root.PersistentFlags().BoolVarP(&util.BatchMode, "batch-mode", "B", true,
```
1. Rename to "batch mode" so we can disable all (future) reactive operations, not just output
2. Change naming style to kebab-case so it's consistent with other flags.
3. Use `-B` as the shorthand flag as it's common in other softwares.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-infra-e2e] fgksgf commented on a diff in pull request #92: Add `batchOutput` config to reduce outputs
Posted by GitBox <gi...@apache.org>.
fgksgf commented on code in PR #92:
URL: https://github.com/apache/skywalking-infra-e2e/pull/92#discussion_r1016605284
##########
docs/en/setup/Configuration-File.md:
##########
@@ -161,6 +161,7 @@ verify:
interval: 10s # the interval between two attempts, e.g. 10s, 1m.
fail-fast: true # when a case fails, whether to stop verifying other cases. This property defaults to true.
concurrency: false # whether to verify cases concurrently. This property defaults to false.
+ batchOutput: true # whether to output the result of verify cases in batch. This property defaults to true.
Review Comment:
ok
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-infra-e2e] fgksgf commented on pull request #92: Add `batchOutput` config to reduce outputs
Posted by GitBox <gi...@apache.org>.
fgksgf commented on PR #92:
URL: https://github.com/apache/skywalking-infra-e2e/pull/92#issuecomment-1314687309
> Thanks very much! Are you interesting in upgrading the main repo and add the batch mode there in CI?
Ok
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-infra-e2e] kezhenxu94 commented on a diff in pull request #92: Add `batchOutput` config to reduce outputs
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on code in PR #92:
URL: https://github.com/apache/skywalking-infra-e2e/pull/92#discussion_r1014839079
##########
docs/en/setup/Configuration-File.md:
##########
@@ -161,6 +161,7 @@ verify:
interval: 10s # the interval between two attempts, e.g. 10s, 1m.
fail-fast: true # when a case fails, whether to stop verifying other cases. This property defaults to true.
concurrency: false # whether to verify cases concurrently. This property defaults to false.
+ batchOutput: true # whether to output the result of verify cases in batch. This property defaults to true.
Review Comment:
Hi, I'd move this to command line options, not in configuration file, otherwise we have to modify the configuration file when running locally and revert back when submit the codes into codebase.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-infra-e2e] github-actions[bot] commented on a diff in pull request #92: Add `batchOutput` config to reduce outputs
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on code in PR #92:
URL: https://github.com/apache/skywalking-infra-e2e/pull/92#discussion_r1014837649
##########
pkg/output/printer.go:
##########
@@ -0,0 +1,82 @@
+package output
Review Comment:
```suggestion
//
// Licensed to Apache Software Foundation (ASF) under one or more contributor
// license agreements. See the NOTICE file distributed with
// this work for additional information regarding copyright
// ownership. Apache Software Foundation (ASF) licenses this file to you under
// the Apache License, Version 2.0 (the "License"); you may
// not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
package output
```
<!-- license-eye hidden identification -->
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org