Thank you for the update and all the work done! 👏🏻 On Sat, 26 Jul 2025 at 15:26, Maxim Muzafarov <mmu...@apache.org> wrote:
> Hello everyone, > > > Finally, we migrated all the commands from airline to *picocli* and > the changes have been merged into the trunk branch. These commands are > backward compatible with everything we had before the patch. > > To ensure adequate compatibility, we have also significantly increased > the number of tests for the nodetool commands. Additionally, a new > environment variable, -Dcassandra.cli.layout=picocli, has been added > to utilize picocli's command help representation. You can view the new > layout by setting the corresponding variable: > > ./bin/nodetool help snapshot -Dcassandra.cli.layout=picocli > > > There are also a few follow-ups available to take full advantage of > the new framework, such as command autocompletion etc. > > https://issues.apache.org/jira/browse/CASSANDRA-20790 > https://issues.apache.org/jira/browse/CASSANDRA-20791 > https://issues.apache.org/jira/browse/CASSANDRA-20792 > https://issues.apache.org/jira/browse/CASSANDRA-20793 > https://issues.apache.org/jira/browse/CASSANDRA-20794 > > This also unlocks the next steps, such as the CQL Management API, > unifying the CQL and MBean interfaces so that the commands will look > the same, or generating the OpenAPI specs on top, aligning everything > out of the box. > > > Please let me know if you encounter any issues, and I will fix them asap. > > On Tue, 13 May 2025 at 11:17, Dmitry Konstantinov <netud...@gmail.com> > wrote: > > > > Hi Maxim, I will take a look too. > > > > Regards, > > Dmitry > > > > On Tue, 13 May 2025 at 08:59, Štefan Miklošovič <smikloso...@apache.org> > wrote: > >> > >> Hi Maxim, > >> > >> I took yet another look after my initial review some time ago and I > still do not see any issues with it. > >> > >> I appreciate that by default it behaves exactly the same way as before > and one has to just flip a switch (env property / system property) to start > to use another layout. > >> > >> Arguments / parameters are behaving the same way as well, no matter if > picocli or legacy airline is used which is just great as the end user does > not notice anything. > >> > >> I would just stay on legacy layout for a while and if no other issues > are found we might contemplate about making the switch but I think we > should just go over some phase of having legacy as default and people might > opt-in into new stuff (that's what I will be doing at least). > >> > >> Not sure what needs to happen in order to merge it, I think one more > committer taking a look would be great. > >> > >> Regards > >> > >> On Fri, May 9, 2025 at 9:45 AM Maxim Muzafarov <mmu...@apache.org> > wrote: > >>> > >>> Hello everyone, > >>> > >>> The commands have been migrated to picocli and are ready for review, > >>> and we need a second committer to review them. > >>> Would anyone be able to help? > >>> > >>> Key points: > >>> - All the commands are backwards-compatible with everything we have in > >>> the trunk now (including the accord commands); > >>> - The commands help output also match the trunk (no difference from > >>> the UX point of view); > >>> - Test coverage has also been significantly increased (most of the > >>> changes are new tests); > >>> > >>> https://github.com/apache/cassandra/pull/2497/files > >>> https://issues.apache.org/jira/browse/CASSANDRA-17445 > >>> > >>> On Mon, 15 Jul 2024 at 20:53, Maxim Muzafarov <mmu...@apache.org> > wrote: > >>> > > >>> > Hello everyone, > >>> > > >>> > > >>> > I want to continue the discussion that was originally started here > >>> > [2], however, it's better to move it to a new thread with an > >>> > appropriate title, so that everyone is aware of the replacement > >>> > library we're trying to agree on. > >>> > > >>> > The question is: > >>> > Does everyone agree with using Picocli as an airlift/airline > >>> > replacement for our cli tools? > >>> > The prototype to look at is here [1]. > >>> > > >>> > > >>> > The reasons are as follows: > >>> > > >>> > Why to replace? > >>> > > >>> > There are several cli tools that rely on the airlift/airline library > >>> > to mark up the commands: NodeTool, JMXTool, FullQueryLogTool, > >>> > CompactionStress (with the size of the NodeTool dominating the rest > of > >>> > the tools). The airline is no longer maintained, so we will have to > >>> > update it sooner or later anyway. > >>> > > >>> > > >>> > What criteria? > >>> > > >>> > Before we dive into the pros and cons of each candidate, I think we > >>> > have to formulate criteria for the libraries we are considering, > based > >>> > on what we already have in the source code (from Cassandra's > >>> > perspective). This in itself limits the libraries we can consider. > >>> > > >>> > Criteria can be as follows: > >>> > - Library licensing, including risks that it may change in the future > >>> > (the asf libs are the safest for us from this perspective); > >>> > - Similarity of library design (to the airline). This means that the > >>> > closer the libraries are, the easier it is to migrate to them, and > the > >>> > easier it is to get guarantees that we haven't broken anything. The > >>> > further away the libraries are, the more extra code and testing we > >>> > need; > >>> > - Backward compatibility. The ideal case is where the user doesn't > >>> > even notice that a different library is being used under the hood. > >>> > This includes both the help output and command output. > >>> > > >>> > Of course, all libraries need to be known and well-maintained. > >>> > > >>> > What candidates? > >>> > > >>> > > >>> > Picocli > >>> > https://picocli.info/ > >>> > > >>> > This is the well-known cli library under the Apache 2.0 license, > which > >>> > is similar to what we have in source code right now. This also means > >>> > that the amount of changes (despite the number of the commands) > >>> > required to migrate what we have is quite small. > >>> > In particular, I would like to point out that: > >>> > - It allows us to unbind the jmx-specific command options from the > >>> > commands themselves, so that they can be reused in other APIs (my > >>> > goal); > >>> > - We can customize the help output so that the user doesn't notice > >>> > anything while using of the nodetool; > >>> > - The cli parser is the same as what we now do with cli arguments. > >>> > > >>> > This makes the library a good candidate, but leaves open the question > >>> > of changing the license of the lib in the future. However, these > risks > >>> > are relatively small because the CLI library is not a monetizable > >>> > thing, as I believe. We can also mitigate the risks copying the lib > to > >>> > sources, as it mentioned here: > >>> > https://picocli.info/#_getting_started > >>> > > >>> > > >>> > commons-cli > >>> > https://commons.apache.org/proper/commons-cli/ > >>> > > >>> > In terms of licenses, it is the easiest candidate for us to use as > >>> > it's under the asf, and in fact the library is already used in e.g. > >>> > BulkLoader, SSTableExpoert. > >>> > However, I'd like to point out the following disadvantages the > library > >>> > has for our case: > >>> > - This is not a drop-in replacement for the airline commands, as the > >>> > lib does not have annotation for markup commands. We have to flesh > out > >>> > all the options we have as java classes, or create our owns; > >>> > - Subcommands have to be supported manually, which requires extra > >>> > effort to adopt the cli parser (correct me if I'm wrong here). We > have > >>> > at least several subcommands in the NodeTool e.g. cms describe, cms > >>> > snapshot; > >>> > - Apart from parsing the cli arguments, we need to manually > initialize > >>> > the command class and set the input arguments we have. > >>> > > >>> > > >>> > JComannder > >>> > https://jcommander.org/ > >>> > > >>> > The library is licensed under the Apache 2.0 license, so the > situation > >>> > is the same as for Picocli. Here I'd like to point out a few things I > >>> > encountered while prototyping: > >>> > - Unbinding the jmx-specific options from commands is quite tricky > and > >>> > requires touching an internal API (which I won't do). Option > >>> > inheritance is not the way to go if we want to have a clear command > >>> > hierarchy regardless of the API used. > >>> > - We won't be able to inject a Logger (the Output class in terms of > >>> > NodeTool) or other abstractions (e.g. MBeans) directly into the > >>> > commands, because it doesn't support dependency injection. This is > >>> > also part of the activity of reusing the commands in other APIs, for > >>> > instance to execute them via CQL; > >>> > > >>> > More basic in comparison to the Picocli, focusing primarily on simple > >>> > annotation-based parsing and subcommands, and won't allow us to reuse > >>> > the commands outside of the cli. > >>> > > >>> > > >>> > airline2 > >>> > https://github.com/rvesse/airline > >>> > > >>> > The library is licensed under the Apache 2.0 license, and this is an > >>> > attempt to rebuild the original airline library. Currently, this is > >>> > not a drop-in replacement, as it has some minor API differences from > >>> > the original library. It is also not a better choice for us, as it > has > >>> > the same drawbacks I mentioned for the previous alternatives, e.g. > not > >>> > possible to unbind the specific options from the command and use them > >>> > only when commands are called from the cli. > >>> > > >>> > > >>> > > >>> > > >>> > [1] https://github.com/apache/cassandra/pull/2497/files > >>> > [2] https://lists.apache.org/thread/m9wfb3gdo9s210824c9rm2ojc9qv9412 > > > > > > > > -- > > Dmitry Konstantinov >
