Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: remove verbosity hack for polycli monitor #156

Merged
merged 9 commits into from
Nov 14, 2023

Conversation

leovct
Copy link
Member

@leovct leovct commented Nov 10, 2023

Description

Follow-up of #154.

Two things:

  • The hack worked fine, but it was poorly implemented as @minhd-vu suggested. This PR removes the hack and implements the functionality in a clean manner using cmd.Flags().Lookup("verbosity") in PersistentPreRun.

  • On top of that, if you wanted to override the verbosity value in polycli monitor, it didn't work because it was hardcoded to silent in my previous PR - 🤦 don't know how I missed that.... This PR modifies the implementation and overrides the default verbosity value properly and only if the user has not specified the verbosity via the flag.

Jira / Linear Tickets

x

Testing

Set up the test environment.

# terminal 1
$ make anvil
# terminal 2
$ make fund
$ go run main.go loadtest --mode t --verbosity 600 --requests 100000000 --rate-limit 300 
  • go run main.go monitor --interval 0.01s - no logs + no glitch
  • go run main.go monitor --interval 0.01s --verbosity 400 - logs + glitch
  • go run main.go monitor --interval 0.01s --verbosity 400 &> log.txt - no logs + no glitch

@leovct leovct requested a review from minhd-vu November 10, 2023 08:58
@leovct leovct marked this pull request as ready for review November 10, 2023 09:16
cmd/monitor/cmd.go Outdated Show resolved Hide resolved
@leovct leovct merged commit a49e238 into main Nov 14, 2023
6 checks passed
@leovct leovct deleted the chore/remove-polycli-monitor-hack branch November 14, 2023 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants