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

feat(cli): add iroha transaction get and other important commands #5289

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

s8sato
Copy link
Contributor

@s8sato s8sato commented Jan 27, 2025

Context

Implements essential but previously missing commands in the client CLI. See Command Line Help

Migration Guide

Some commands are changed in usage or behavior:

  • iroha json query -> iroha query stdin
  • iroha json transaction -> iroha transaction stdin
  • iroha wasm -> iroha transaction wasm
  • iroha * metadata -> iroha * meta
  • iroha asset
    • get-key-value -> getkv
    • set-key-value -> setkv
    • remove-key-value -> removekv
  • iroha events
    • data -> state
    • transaction-pipeline -> transaction
    • block-pipeline -> block
    • execute-trigger -> trigger-execute
    • trigger-completed -> trigger-complete
  • iroha * list all -> iroha * list all --verbose
  • iroha * list all lists only IDs and is intended to be used in combination with iroha * get --id, which retrieves the details of a single entry.

Checklist

  • I've read CONTRIBUTING.md.
  • All review comments have been resolved.
  • All CI checks pass.

scripts/tests/tick.json Outdated Show resolved Hide resolved
crates/iroha_cli/README.md Outdated Show resolved Hide resolved
| `peer` | Execute commands related to peer administration and networking |
| `wasm` | Execute commands related to WASM |
| `help` | Print the help message for `iroha` and/or the current subcommand other than `help` subcommand |
See [Command-Line Help](CommandLineHelp.md).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Delegate the usage description to an auto-generated Command-Line Help

Comment on lines +1 to +5
{
"sumeragi": {
"block_time_ms": 2001
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

snake_case for consistency with FindParameters query result to cut and reuse it as SetParameter input

Copy link
Contributor Author

@s8sato s8sato Feb 6, 2025

Choose a reason for hiding this comment

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

If SDKs already implement SetParameter I'd revert this change @0x009922 @mversic

    #[serde(rename_all = "snake_case")]
    pub enum Parameter {

Copy link
Contributor

Choose a reason for hiding this comment

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

Iroha JS supports this, but I am not sure I understand your point and what's the issue here. Could you help me understand?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is, for example,

{
  "Sumeragi": {
    "BlockTimeMs": 2001
  }
}

will be failing to deserialize to SumeragiParameter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some #[serde(...)] attributes don't change the schema, so it seems easy for them to silently break the API

@s8sato s8sato added the CLI label Jan 27, 2025
@s8sato s8sato force-pushed the feat/complete_cli branch from 4f5d6d5 to 1c5adf3 Compare February 4, 2025 17:22
Signed-off-by: Shunkichi Sato <[email protected]>
aoyako
aoyako previously approved these changes Feb 5, 2025
@s8sato s8sato force-pushed the feat/complete_cli branch from 0093bea to 8bcdcac Compare February 7, 2025 07:30
Comment on lines +5 to +9
## [2.0.0-rc.1.1] - 2025-02-08

### Added

- add `iroha transaction get` and other important commands (#5289)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if this follows the backporting workflow @mversic

Also,

  1. Update CHANGELOG in a separate commit (we must have an entry for patch versions in the main branch as well)

Why do we update the CHANGELOG.md in the main branch for every minor version change due to backporting? Wouldn't it be sufficient to update it only in the release branches?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI Enhancement New feature or request
Projects
None yet
4 participants