chore: transform `url` arg into a flag #138

leovct · 2023-10-18T12:33:00Z

Description

🚨 Breaking change 🚨

It's just a suggestion, no problem if we don't merge it :)

This pull request enhances the loadtest, loadtest uniswapv3, monitor and rpcfuzz commands by transforming the url argument into a more flexible and user-friendly flag called rpc-url. With this update, users can now set the URL as a flag, offering greater control and simplicity in command usage, following the standards employed by other tools such as cast.

By default, the rpc-url flag is set to http://localhost:8545.

$ polycli loadtest --help | grep rpc-url
  -r, --rpc-url string                        The RPC endpoint url (default "http://localhost:8545")

It's now possible to run polycli loadtest. If you were specifying the url as an argument previously, you can switch from polycli loadtest <url> to polycli loadtest --rpc-url <url> or polycli loadtest -r <url>.

Other

Move most of the logic related to cobra.Command to a separate file cmd.go or app.go for each cmd.
Move logic related to flags in checkFlags and logic responsible for running the cmd in a specific func.
Add a method to validate urls in util, to be reused in other commands also.

For example, here's how the monitor cmd looks now.

// cmd/monitor/cmd.go

// MonitorCmd represents the monitor command
var MonitorCmd = &cobra.Command{
	Use:   "monitor",
	Short: "Monitor blocks using a JSON-RPC endpoint.",
	Long:  usage,
	Args:  cobra.NoArgs,
	PreRunE: func(cmd *cobra.Command, args []string) error {
		return checkFlags()
	},
	RunE: func(cmd *cobra.Command, args []string) error {
		return monitor(cmd.Context())
	},
}

Jira / Linear Tickets

DVT-1020

Testing

x

leovct · 2023-10-19T11:22:58Z

If we end up merging this PR, we should make sure we update the polycli loadtest uniswapv3 cmd (see #137)

leovct · 2023-10-20T17:19:40Z

If we end up merging this PR, we should make sure we update the polycli loadtest uniswapv3 cmd (see #137)

Implemented in the PR ✅

rebelArtists · 2023-10-20T17:31:36Z

cmd/loadtest/uniswapv3.go

-func validateUrl(input string) (*url.URL, error) {
-	url, err := url.Parse(input)
-	if err != nil {
-		log.Error().Err(err).Msg("Unable to parse url input error")
-		return nil, err
-	}
-
-	if url.Scheme == "" {
-		return nil, errors.New("the scheme has not been specified")
-	}
-	switch url.Scheme {
-	case "http", "https", "ws", "wss":
-		return url, nil
-	default:
-		return nil, fmt.Errorf("the scheme %s is not supported", url.Scheme)
-	}
-}
-


nice simplification here

rebelArtists · 2023-10-20T17:35:03Z

doc/polycli_loadtest.md

@@ -14,7 +14,7 @@
 Run a generic load test against an Eth/EVM style JSON-RPC endpoint.

 ```bash
-polycli loadtest url [flags]
+polycli loadtest [flags]


nit: potentially add comment here that rpc-url default is localhost

rebelArtists · 2023-10-20T17:35:58Z

doc/polycli_loadtest_uniswapv3.md

@@ -98,6 +98,7 @@ The command also inherits flags from parent commands.
      --private-key string                         The hex encoded private key that we'll use to send transactions (default "42b6e34dc21598a807dc19d7784c71b2a7a01f6480dc6f58258f78e539f1a1fa")
      --rate-limit float                           An overall limit to the number of requests per second. Give a number less than zero to remove this limit all together (default 4)
  -n, --requests int                               Number of requests to perform for the benchmarking session. The default is to just perform a single request which usually leads to non-representative benchmarking results. (default 1)
+  -r, --rpc-url string                             The RPC endpoint url (default "http://localhost:8545")


nvm, this is perfect

rebelArtists · 2023-10-20T17:37:35Z

scripts/clients.mk

@@ -37,10 +37,10 @@ geth-loadtest: build fund ## Run loadtest against an EVM/Geth chain.
 	sleep 5
 	$(BUILD_DIR)/$(BIN_NAME) loadtest \
 		--verbosity 700 \
+		--rpc-url http://127.0.0.1:$(PORT) \


nit: potentially make host configurable

yeah we could, this is just a handy command to trigger load test in ci :)

rebelArtists

looks great, couple nits but not a blocker

leovct added 5 commits October 18, 2023 14:22

chore(loadtest): url is now a flag instead of an arg

d1710cf

chore: rename url flag into rpc-endpoint like cast

520bcf8

chore: gen doc

9b94d0b

chore: update doc and test script

ebbe86e

chore: rename rpc-endpoint into rpc-url

d1404a6

leovct requested a review from a team October 19, 2023 11:20

leovct marked this pull request as ready for review October 19, 2023 11:20

leovct added 3 commits October 19, 2023 14:12

chore: move ValidateUrl to util pkg

ed8b611

chore: same thing for monitor cmd

bdc3c52

chore: update doc

0a06b65

leovct changed the title ~~chore(loadtest): remove url arg~~ chore: transform url arg into a flag Oct 19, 2023

leovct and others added 9 commits October 19, 2023 15:22

chore: same thing for rpcfuzz

df37cb6

chore: clean up

c564817

chore: more clean up

5ff1dba

Merge branch 'main' into chore/remove-url-arg

50cba64

Merge remote-tracking branch 'origin/main' into chore/remove-url-arg

8051fa7

chore: remove url arg from uniswapv3 loadtest cmd

276b312

chore: gen doc

ff866ec

chore: add zerolog setup

de07998

chore: update usage

8ac42b9

rebelArtists reviewed Oct 20, 2023

View reviewed changes

rebelArtists approved these changes Oct 20, 2023

View reviewed changes

leovct merged commit 6d912a6 into main Oct 21, 2023
5 checks passed

leovct deleted the chore/remove-url-arg branch October 21, 2023 10:40

leovct mentioned this pull request Oct 23, 2023

doc: update loadtest uniswapv3 usage #141

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: transform `url` arg into a flag #138

chore: transform `url` arg into a flag #138

leovct commented Oct 18, 2023 •

edited

Loading

leovct commented Oct 19, 2023

leovct commented Oct 20, 2023

rebelArtists Oct 20, 2023

rebelArtists Oct 20, 2023

rebelArtists Oct 20, 2023

rebelArtists Oct 20, 2023

leovct Oct 21, 2023

rebelArtists left a comment

chore: transform url arg into a flag #138

chore: transform url arg into a flag #138

Conversation

leovct commented Oct 18, 2023 • edited Loading

Description

Other

Jira / Linear Tickets

Testing

leovct commented Oct 19, 2023

leovct commented Oct 20, 2023

rebelArtists Oct 20, 2023

Choose a reason for hiding this comment

rebelArtists Oct 20, 2023

Choose a reason for hiding this comment

rebelArtists Oct 20, 2023

Choose a reason for hiding this comment

rebelArtists Oct 20, 2023

Choose a reason for hiding this comment

leovct Oct 21, 2023

Choose a reason for hiding this comment

rebelArtists left a comment

Choose a reason for hiding this comment

chore: transform `url` arg into a flag #138

chore: transform `url` arg into a flag #138

leovct commented Oct 18, 2023 •

edited

Loading