[
https://issues.apache.org/jira/browse/CASSANDRA-16818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17390026#comment-17390026
]
Andres de la Peña commented on CASSANDRA-16818:
-----------------------------------------------
Thanks for the review. I also think that improving the description and error
messages is better, since the syntax itselt isn't wrong and it's only the
description and documentation what is wrong and/or it's a bit misleading. Also,
changing the syntax can lead to compatibility issues.
{quote}I am wondering, shall we add a test covering the specific case?
{quote}
The patch already contains four new tests in {{SSTableExportTest}} covering
multiple keys and arguments in the wrong order for both arguments, do you mean
any additional tests beyond those?
{quote}I left two super small comments on your commit.
{quote}
Thanks for the comments, I have added the {{SuppressWarnings}} annotation and
modified the checks in {{assertPostTestEnv}} so they are in line with other
tests. Here are the PRs for 4.0 and trunk:
||PR||CI||
|[4.0|https://github.com/apache/cassandra/pull/1121]|[j8|https://app.circleci.com/pipelines/github/adelapena/cassandra/708/workflows/eed7fc79-51b1-4f1c-8da5-8255cd9cf49f]
[j11|https://app.circleci.com/pipelines/github/adelapena/cassandra/708/workflows/8352fab0-d0c4-42f6-9dc7-db2fcef6b810]|
|[trunk|https://github.com/apache/cassandra/pull/1122]|[j8|https://app.circleci.com/pipelines/github/adelapena/cassandra/707/workflows/97a532c1-3415-484e-8cf2-1d59fe664825]
[j11|https://app.circleci.com/pipelines/github/adelapena/cassandra/707/workflows/738901f7-61f6-41fb-a257-4fe9b3350f43]|
The CI runs contain 100 repeated runs of {{SSTableExportTest}}.
I have also added some fixes to the [documentation for
{{sstabledump}}|https://github.com/apache/cassandra/blob/b470459067641892855750c04e0b098af86e1e7b/doc/source/tools/sstable/sstabledump.rst].
I haven't prepared PRs for 3.0 nor 3.11 because IMO this isn't a very serious
issue and we don't have either the test tooling or the docs in those branches,
wdyt?
> Fix usage instructions about sstabledump -k and -x options
> ----------------------------------------------------------
>
> Key: CASSANDRA-16818
> URL: https://issues.apache.org/jira/browse/CASSANDRA-16818
> Project: Cassandra
> Issue Type: Bug
> Components: Tool/sstable
> Reporter: Andres de la Peña
> Assignee: Andres de la Peña
> Priority: Normal
> Fix For: 3.0.x, 3.11.x, 4.0.x, 4.x
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> The options {{-k}} and {{-x}} of {{sstabledump}} admit multiple arguments, so
> users can include or exclude multiple partitions. The intended usage is, for
> example:
> {code:java}
> $ sstablepartitions <sstable_path> -k 1
> $ sstablepartitions <sstable_path> -k 1 2 3
> {code}
> However, the following command will fail:
> {code:java}
> $ sstablepartitions -k 1 <sstable_path>
> You must supply exactly one sstable
> usage: sstabledump <sstable file path> <options>
> Dump contents of given SSTable to standard output in JSON format.
> -d CQL row per line internal representation
> -e enumerate partition keys only
> -k <arg> Partition key
> -t Print raw timestamps instead of iso8601 date strings
> -x <arg> Excluded partition key
> {code}
> This command fails because the sstable path is considered a fourth partition
> key, and the mandatory argument for the sstable path is missed. While I think
> this behaviour is correct, it can be a bit confusing for users, especially
> when the information about usage describes both {{-k}} and {{-x}} as
> single-argument.
> I think that at least we should fix the description of the options to
> indicate that there could be multiple included/excluded keys, and probably
> improve the message about the missing sstable path when those options are
> used.
> Alternatively we could modify the options to have a single argument and allow
> to repeat them, so we could accept things like:
> {code:java}
> $ sstablepartitions -k 1 <sstable_path>
> $ sstablepartitions -k 1 -k 2 -k 3 <sstable_path>
> $ sstablepartitions <sstable_path> -k 1 -k 2 -k 3
> {code}
> The main downside of the latter approach is that the change in the syntax of
> the command might cause compatibility issues. Also we would need to upgrade
> {{commons-cli}} to at least 1.2 due to CLI-137.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
