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

tools/unitctl: implement application subcommand #1323

Merged
merged 1 commit into from
Jun 18, 2024

Conversation

avahahn
Copy link
Contributor

@avahahn avahahn commented Jun 17, 2024

This MR addresses #1280 and #1279 by providing a subcommand unitctl app that provides two options:

  • unitctl app list: returns a list of running applications
$ unitctl app list -t json-pretty
{
  "wasm": {
    "type": "wasm-wasi-component",
    "component": "/www/wasmapp-proxy-component.wasm"
  }
}
  • unitctl app <NAME> reload: reloads an application by name
$ unitctl app reload wasm -t json-pretty
{
  "success": "Ok"
}

This MR also adds missing wasm and wasi application schemas to the OpenAPI spec.

Copy link
Contributor

@javorszky javorszky left a comment

Choose a reason for hiding this comment

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

Two questions, but both are non-blocking. Looks good.

Installation on a mac M series chip needs openjdk rather than java from java.com (latest is JRE 8 v 411 at the time of writing, only supports class file version 52.0) as openapi files have class file version 55.0

tools/unitctl/GNUmakefile Show resolved Hide resolved
tools/unitctl/README.md Outdated Show resolved Hide resolved
@@ -114,6 +114,8 @@ pub(crate) enum Commands {
)]
output_format: OutputFormat,
},
#[command(about = "List all configured UNIT applications")]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
#[command(about = "List all configured UNIT applications")]
#[command(about = "List all configured Unit applications")]

...and we should probably check that we're consistent elsewhere in unitctl.

Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't say it's a suggestion!

Unit is never spelled UNIT (it's not an acronym)

Copy link
Collaborator

@callahad callahad left a comment

Choose a reason for hiding this comment

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

Some nits around prose, but otherwise lgtm.

I really wish the whitespace / rustfmt changes were broken into their own commits. Next time? 🙏

* application subcommand UI schema
* application subcommand handler
* additions to unit-client-rs to expose application API
* elaborate on OpenAPI error handling
* adds wasm and wasi app schemas to OpenAPI Schema
* updates tools/unitctl OpenAPI library
* many linter fixes
* README.md updates

Signed-off-by: Ava Hahn <a.hahn@f5.com>
@avahahn avahahn merged commit e0c15ae into nginx:master Jun 18, 2024
8 checks passed
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.

4 participants