unitctl: match readme with current state of unitctl output

javorszky commented 1 week ago

This PR does the following:

changes capitalisation of Unit to be consistent with style guide
adds the export subcommand to the readme
matches the CONTROL_SOCKET_ADDRESSES to current cli output

callahad commented 1 week ago

Instead of pluralizing CONTROL_SOCKET_ADDRESS to match CLI output... should we change the CLI itself so the variable is printed in the singular?

The plural there confuses me, as I don't see where in the code we actually allow multiple sockets to be specified as a single CLI arg. Maybe you can pass -s multiple times to build up the array, but each time you're passing a singular socket. Hm.

javorszky commented 1 week ago

technically the option is a Option<Vec>, though the parse argument will only return one of them by the looks of it: https://github.com/nginx/unit/blob/master/tools/unitctl/unit-client-rs/src/control_socket_address.rs#L88

I think this is an @avahahn question, half the code suggests it should be plural, other half suggests it should be singular.

callahad commented 1 week ago

Yeah, src/wait.rs does iterate through multiple sockets, so it makes sense. Just think we need to change what's displayed in the help output.

Looks like .value_name("CONTROL_SOCKET_ADDRESS") would do that in clap's builder API, but it's not clear to me how to use that from the derive interface. Might be as simple as value_name = "CONTROL_SOCKET_ADDRESS", in the arg block?

callahad commented 1 week ago

Maybe you can pass -s multiple times to build up the array, but each time you're passing a singular socket. Hm.

I could be talking out of my ass here; haven't actually used clap directly. I should instead ask the question: how do you pass multiple sockets to unitctl? The docs for num_args warn against accepting multiple args for a single flag, and instead suggest:

Use a delimiter between values with Arg::value_delimiter

Require a flag occurrence per value with ArgAction::Append

Require positional arguments to appear after -- with Arg::last

...I want the append behavior, but (1) what are we doing right now, and (2) if it's not append, how do we switch to that?

callahad commented 1 week ago

The docs for Arg Types suggest that Append is the default action for Vec types.

avahahn commented 1 week ago

I think this is an @avahahn question, half the code suggests it should be plural, other half suggests it should be singular.

The user will specify each individual control socket per -s flag. See this PR for more information: https://github.com/nginx/unit/pull/1350

...I want the append behavior, but (1) what are we doing right now, and (2) if it's not append, how do we switch to that?

As you can see at the bottom of this readme section, the flag and its value can be provided multiple times for the append behavior: https://github.com/nginx/unit/tree/master/tools/unitctl#lists-active-applications-and-provides-means-to-restart-them

javorszky commented 1 week ago

outstanding: control socket addresses:

should be singular
should have help text that it can be specified multiple times

javorszky commented 1 week ago

All right, I updated the control socket address option, and made sure this changeset is over the current master branch.

ac000 commented 1 week ago

Rebase with master
Bunch more s/UNIT/Unit/

$ git range-diff 0e70d266...7c48546a
 -:  -------- >  1:  50b1aca3 python: Don't decrement a reference to a borrowed object
 -:  -------- >  2:  3c563849 unitctl: Don't track unit-openapi/.openapi-generator/
 -:  -------- >  3:  63148a31 tools/unitctl: whitespace fixes
 -:  -------- >  4:  5e8a6893 tools/unitctl: rename app -> apps, fix readme
 -:  -------- >  5:  f2e05bc7 docs: remove security.txt file
 -:  -------- >  6:  cc863e15 docs: add SECURITY.md
 1:  1e01ab19 !  7:  0dcd3a91 tools/unitctl: rename UNIT -> Unit
    @@ Commit message
         caps.

         Signed-off-by: Gabor Javorszky <g.javorszky@f5.com>
    +    [ A bunch more s/UNIT/Unit/ - Andrew ]
    +    Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
    +
    + ## tools/unitctl/GNUmakefile ##
    +@@ tools/unitctl/GNUmakefile: manpage: target/man/$(OUTPUT_BINARY).1.gz ## Builds man page
    + .openapi_cache:
    +   $Q mkdir -p $@
    + 
    +-## Generate (or regenerate) UNIT API access code via a OpenAPI spec
    ++## Generate (or regenerate) Unit API access code via a OpenAPI spec
    + .PHONY: openapi-generate
    + openapi-generate: .openapi_cache
    +   $Q if [ ! -f "$(CURDIR)/unit-openapi/src/models/mod.rs" ]; then
    +-          echo "$(M) generating UNIT API access code via a OpenAPI spec"
    ++          echo "$(M) generating Unit API access code via a OpenAPI spec"
    +           OPENAPI_GENERATOR_VERSION="$(OPENAPI_GENERATOR_VERSION)" \
    +           OPENAPI_GENERATOR_DOWNLOAD_CACHE_DIR="$(CURDIR)/.openapi_cache" \
    +           $(CURDIR)/build/openapi-generator-cli.sh \
    +
    + ## tools/unitctl/man/unitctl.1 ##
    +@@
    + .\"
    + .TH UNITCTL "1" "2022-12-29" "%%VERSION%%" "unitctl"
    + .SH NAME
    +-unitctl \- NGINX UNIT Control Utility
    ++unitctl \- NGINX Unit Control Utility
    + .SH SYNOPSIS
    + unitctl [\fI\,FLAGS\/\fR] [\fI\,OPTIONS\/\fR] [\fI\,FILE\/\fR]...
    + .SH DESCRIPTION
    +
 -:  -------- >  1:  50b1aca3 python: Don't decrement a reference to a borrowed object
 -:  -------- >  2:  3c563849 unitctl: Don't track unit-openapi/.openapi-generator/
 -:  -------- >  3:  63148a31 tools/unitctl: whitespace fixes
 -:  -------- >  4:  5e8a6893 tools/unitctl: rename app -> apps, fix readme
 -:  -------- >  5:  f2e05bc7 docs: remove security.txt file
 -:  -------- >  6:  cc863e15 docs: add SECURITY.md
 1:  1e01ab19 !  7:  0dcd3a91 tools/unitctl: rename UNIT -> Unit
    @@ Commit message
         caps.

         Signed-off-by: Gabor Javorszky <g.javorszky@f5.com>
    +    [ A bunch more s/UNIT/Unit/ - Andrew ]
    +    Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
    +
    + ## tools/unitctl/GNUmakefile ##
    +@@ tools/unitctl/GNUmakefile: manpage: target/man/$(OUTPUT_BINARY).1.gz ## Builds man page
    + .openapi_cache:
    +   $Q mkdir -p $@
    + 
    +-## Generate (or regenerate) UNIT API access code via a OpenAPI spec
    ++## Generate (or regenerate) Unit API access code via a OpenAPI spec
    + .PHONY: openapi-generate
    + openapi-generate: .openapi_cache
    +   $Q if [ ! -f "$(CURDIR)/unit-openapi/src/models/mod.rs" ]; then
    +-          echo "$(M) generating UNIT API access code via a OpenAPI spec"
    ++          echo "$(M) generating Unit API access code via a OpenAPI spec"
    +           OPENAPI_GENERATOR_VERSION="$(OPENAPI_GENERATOR_VERSION)" \
    +           OPENAPI_GENERATOR_DOWNLOAD_CACHE_DIR="$(CURDIR)/.openapi_cache" \
    +           $(CURDIR)/build/openapi-generator-cli.sh \
    +
    + ## tools/unitctl/man/unitctl.1 ##
    +@@
    + .\"
    + .TH UNITCTL "1" "2022-12-29" "%%VERSION%%" "unitctl"
    + .SH NAME
    +-unitctl \- NGINX UNIT Control Utility
    ++unitctl \- NGINX Unit Control Utility
    + .SH SYNOPSIS
    + unitctl [\fI\,FLAGS\/\fR] [\fI\,OPTIONS\/\fR] [\fI\,FILE\/\fR]...
    + .SH DESCRIPTION
    +
    + ## tools/unitctl/pkg/brew/unitctl.rb ##
    +@@
    + class Unitctl < Formula
    +-  desc "CLI interface to the NGINX UNIT Control API"
    ++  desc "CLI interface to the NGINX Unit Control API"
    +   homepage "https://github.com/nginxinc/unit-rust-sdk"
    +   version "0.3.0"
    +   package_name = "unitctl"
    +
    + ## tools/unitctl/pkg/brew/unitctl.rb.template ##
    +@@
    + class Unitctl < Formula
    +-  desc "CLI interface to the NGINX UNIT Control API"
    ++  desc "CLI interface to the NGINX Unit Control API"
    +   homepage "https://github.com/nginxinc/unit-rust-sdk"
    +   version "$VERSION"
    +   package_name = "$PACKAGE_NAME"
    +
    + ## tools/unitctl/unit-client-rs/src/unit_client.rs ##
    +@@ tools/unitctl/unit-client-rs/src/unit_client.rs: use unit_openapi::apis::{
    + };
    + use unit_openapi::models::{ConfigApplication, ConfigListener, Status};
    + 
    +-const USER_AGENT: &str = concat!("UNIT CLI/", env!("CARGO_PKG_VERSION"), "/rust");
    ++const USER_AGENT: &str = concat!("Unit CLI/", env!("CARGO_PKG_VERSION"), "/rust");
    + 
    + custom_error! {pub UnitClientError
    +     OpenAPIError { source: OpenAPIError } = "OpenAPI error",
    +@@ tools/unitctl/unit-client-rs/src/unit_client.rs: impl UnitClient {
    +         }
    +     }
    + 
    +-    /// Sends a request to UNIT and deserializes the JSON response body into the value of type `RESPONSE`.
    ++    /// Sends a request to Unit and deserializes the JSON response body into the value of type `RESPONSE`.
    +     pub async fn send_request_and_deserialize_response<RESPONSE: for<'de> serde::Deserialize<'de>>(
    +         &self,
    +         mut request: Request<Body>,

      ## tools/unitctl/unitctl/Cargo.toml ##
     @@
    @@ tools/unitctl/unitctl/src/cmd/edit.rs: pub(crate) async fn cmd(cli: &UnitCtl, ou
      ## tools/unitctl/unitctl/src/unitctl.rs ##
     @@ tools/unitctl/unitctl/src/unitctl.rs: pub(crate) enum Commands {
          #[command(about = "List all configured Unit applications")]
    -     App(ApplicationArgs),
    +     Apps(ApplicationArgs),

     -    #[command(about = "Export the current configuration of UNIT")]
     +    #[command(about = "Export the current configuration of Unit")]
 2:  4434e102 <  -:  -------- tools/unitctl: add export subcommand to readme
 -:  -------- >  8:  9e5f961b tools/unitctl: add export subcommand to readme
 3:  0e70d266 !  9:  7c48546a tools/unitctl: adjust readme for socket addresses
    @@ Commit message
         CONTROL_SOCKET_ADDRESS as singular.

         Signed-off-by: Gabor Javorszky <g.javorszky@f5.com>
    +    Signed-off-by: Andrew Clayton <a.clayton@nginx.com>

      ## tools/unitctl/README.md ##
     @@ tools/unitctl/README.md: Commands:
    @@ tools/unitctl/unitctl/src/unitctl.rs: pub(crate) struct UnitCtl {
     +        help = "Path (unix:/var/run/unit/control.sock), tcp address with port (127.0.0.1:80), or URL. This flag can be specified multiple times."
          )]
          pub(crate) control_socket_addresses: Option<Vec<ControlSocket>>,
    -     #[arg(
    +

nginx / unit

unitctl: match readme with current state of unitctl output #1425