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

Refactor CLI #154

Merged
merged 1 commit into from
Oct 2, 2022
Merged

Refactor CLI #154

merged 1 commit into from
Oct 2, 2022

Conversation

gzigzigzeo
Copy link
Contributor

What is the purpose of this pull request?

Refactor cli namespace.

What changes did you make? (overview)

  • Implemented declarative CLI args specification using github.com/urfave/cli/v2.
  • cli.Option in style of NATS server configurator.
  • Refactored cli.Runner

Checklist

  • I've added tests for this change
  • I've added a Changelog entry
  • I've updated documentation
NAME:
   anycable-go - AnyCable-Go, The WebSocket server for https://anycable.io

USAGE:
   anycable-go [global options] [arguments...]

VERSION:
   1.2.1-562d4b8

GLOBAL OPTIONS:
   --help, -h     show help (default: false)
   --version, -v  print the version (default: false)

   ADAPTER:
   --broadcast_adapter value  Broadcasting adapter to use (redis, http or nats) (default: "redis") [$ANYCABLE_BROADCAST_ADAPTER]

   ANYCABLE-GO SERVER:
   --health-path value  HTTP health endpoint path (default: "/health") [$ANYCABLE_HEALTH_PATH]
   --host value         Server host (default: "localhost") [$ANYCABLE_HOST]
   --max-conn value     Limit simultaneous server connections (0 – without limit) (default: 0) [$ANYCABLE_MAX_CONN]
   --path value         WebSocket endpoint path (you can specify multiple paths using comma as separator) (default: "/cable") [$ANYCABLE_PATH]
   --port value         Server port (default: 8080) [$PORT, $ANYCABLE_PORT]

   DISCONNECT OPTIONS:
   --disable_disconnect        Disable calling Disconnect callback (default: false) [$ANYCABLE_DISABLE_DISCONNECT]
   --disconnect_rate value     Max number of Disconnect calls per second (default: 100) [$ANYCABLE_DISCONNECT_RATE]
   --disconnect_timeout value  Graceful shutdown timeouts (in seconds) (default: 5) [$ANYCABLE_DISCONNECT_TIMEOUT]

   EMBEDDED NATS SERVICE:
   --mnats_cluster_addr value  NATS cluster service bind address [$ANYCABLE_MNATS_CLUSTER_ADDR]
   --mnats_cluster_name value  NATS cluster name (default: "anycable-cluster") [$ANYCABLE_MNATS_CLUSTER_NAME]
   --mnats_debug               Enable NATS server debug logs. Automatically set to true if --debug is passed. (default: false) [$ANYCABLE_MNATS_DEBUG]
   --mnats_enable              Pass this option to enable embedded NATS server (default: false) [$ANYCABLE_MNATS_ENABLE]
   --mnats_log                 Enable NATS server logs (default: false) [$ANYCABLE_MNATS_LOG]
   --mnats_routes value        Comma separated list of known other cluster service addresses. [$ANYCABLE_MNATS_ROUTES]
   --mnats_service_addr value  NATS server bind address (default: "nats://127.0.0.1:4222") [$ANYCABLE_MNATS_SERVICE_ADDR]
   --mnats_trace               Enable NATS server protocol trace logs. (default: false) [$ANYCABLE_MNATS_TRACE]

Copy link
Member

@palkan palkan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

Left a few minor comments.

Readme.md Outdated
@@ -80,6 +80,7 @@ make prepare
make test-conformance

# Run integration benchmarks
npm i -g websocket-bench
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're using https://github.com/anycable/websocket-bench

Suggested change
npm i -g websocket-bench
go install github.com/anycable/websocket-bench@latest

Makefile Outdated
@@ -118,7 +118,7 @@ test-conformance-http: tmp/anycable-go-test
BUNDLE_GEMFILE=.circleci/Gemfile ANYCABLE_BROADCAST_ADAPTER=http ANYCABLE_HTTP_BROADCAST_SECRET=any_secret bundle exec anyt -c "tmp/anycable-go-test --headers=cookie,x-api-token" --target-url="ws://localhost:8080/cable"

test-conformance-nats: tmp/anycable-go-test
BUNDLE_GEMFILE=.circleci/Gemfile ANYCABLE_BROADCAST_ADAPTER=nats bundle exec anyt -c "tmp/anycable-go-test --headers=cookie,x-api-token" --target-url="ws://localhost:8080/cable"
BUNDLE_GEMFILE=.circleci/Gemfile ANYCABLE_BROADCAST_ADAPTER=nats ANYCABLE_MNATS_ENABLE=true bundle exec anyt -c "tmp/anycable-go-test --headers=cookie,x-api-token" --target-url="ws://localhost:8080/cable"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
BUNDLE_GEMFILE=.circleci/Gemfile ANYCABLE_BROADCAST_ADAPTER=nats ANYCABLE_MNATS_ENABLE=true bundle exec anyt -c "tmp/anycable-go-test --headers=cookie,x-api-token" --target-url="ws://localhost:8080/cable"
BUNDLE_GEMFILE=.circleci/Gemfile ANYCABLE_BROADCAST_ADAPTER=nats bundle exec anyt -c "tmp/anycable-go-test --headers=cookie,x-api-token" --target-url="ws://localhost:8080/cable"

cli/cli_flags.go Outdated
@@ -0,0 +1,521 @@
package cli
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's call this file options.go (to preserve the original name and avoid cli/cli_xx).

@gzigzigzeo gzigzigzeo force-pushed the chore/refactor-cli branch 2 times, most recently from c115903 to 44d56f8 Compare August 12, 2022 15:25
cli/options.go Outdated
Name: "redis_url",
Usage: "Redis url",
Value: c.Redis.URL,
EnvVars: []string{"ANYCABLE_REDIS_URL", "REDIS_URL"},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about prepending prefixed env to the list of passed EnvVars if they defined?

This way, all the prefixed envs would be handled automatically in one place.

@palkan palkan merged commit adc4731 into anycable:master Oct 2, 2022
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