> Once you are happy with your chosen library, we need a DISCUSS thread to add > this new library (current protocol).
Thanks, David. This is a good point, do we need a separate DISCUSS thread or can we just use this one? I'm in favour of keeping the discussion in one place, especially when topics are closely related. I don't think that a separate thread would add extra visibility, but if that is the way the community has adopted - no problem at all, I'll repost. The reasons for replacing the Airlift/Airline [1] with the PicoCli [2] are as follows (in order of priority): 1. The library is under the Apache-2.0 License https://github.com/remkop/picocli?tab=Apache-2.0-1-ov-file#readme 2. The project is active and well-maintained (last release on 8 May 2024) https://github.com/remkop/picocli/releases 3. The library has ZERO dependencies, in some of the cases a single file can just be dropped into the sources (it's even pointed out in the documentation) https://picocli.info/#_add_as_source 4. Compared to the Airlift library, the PicoCLI uses the same markup design concepts, so we don't have to rewrite our command or make complex changes, which in turn minimizes the migration. Is adding the PicoCLI library as a project dependency getting any objections from the Community? Please, share your thoughts. There are a few other alternatives (commons-cli, airline2, jcommander) but they are not as well known and/or not as elegantly suited to our needs based on what we have now. [1] https://github.com/airlift/airline [2] https://github.com/remkop/picocli On Wed, 3 Jul 2024 at 22:27, David Capwell <dcapw...@apple.com> wrote: > > I don't personally think there is a strong need for a feature branch. If it > makes it easy for you, please go ahead with a feature branch. > > > Agree, I don’t see the reason for a feature branch… feature branch just means > the branch lives in apache domain rather than your own fork. You won’t be > able to merge until you are done and you will need to keep rebasing over and > over again. Even if multiple people are working on this you can work in your > fork just fine (assuming you grant permissions). > > Another issue is that feature branches require the same level of commit > process as every other main branch, where as personal branches don’t. This > actually will slow you down as each commit now must be a JIRA, you go through > review of each, must show a success CI, etc. > > Now, if you wish to split this into multiple steps that is fine, but the list > of places is basically node tool (kinda has to go in at once) and small CLIs. > If you wish to migrate the small ones in isolation first, I am cool with > that merging to w/e branch the logic is targeting, but you won’t be able to > break up node tool without breaking everything… but if you did this in your > own fork then no one cares. > > I won’t block a feature branch, but just don’t see a clear “why” and only see > cons. > > We are changing the command markup library, so there are two extra > things to be checked: > - We parse CLI arguments in the same way (as the parser is different > in a new library); > - The command help output is the same so that the user won't see any > difference; > > > Personally I would POC a limited node tool change with JVM dtest as we > require passing the output to the test (the prototypes you listed doesn’t > include JVM Dtest integration). If one library makes this more annoying, > then do we care about fancy new features we don’t use when it makes the > features we do use harder? If you start with the smaller tools first then > spend a ton of time migrating node tool then find JVM dtest is broken, then > you will spend so much more time fixing this, I would strongly recommend > doing some throw away POC to make sure w/e way you go won’t break JVM Dtest’s > node tool support. > > Once you are fine with your selected library, we will need a DISCUSS thread > to add that new library (current protocol). This mostly just makes the pick > more visible, and normally we only check simple things like “are we legally > allowed to use” and “is this project dead?”. > > > On Jul 3, 2024, at 6:06 AM, Maxim Muzafarov <mmu...@apache.org> wrote: > > Thank you all for your comments, > > I want to stress, that these changes won't affect the input/output > formatting of commands, ensuring everything is the same. > > We are changing the command markup library, so there are two extra > things to be checked: > - We parse CLI arguments in the same way (as the parser is different > in a new library); > - The command help output is the same so that the user won't see any > difference; > > Additional tests cover both cases. > > On Mon, 1 Jul 2024 at 20:08, Dinesh Joshi <djo...@apache.org> wrote: > > > I don't personally think there is a strong need for a feature branch. If it > makes it easy for you, please go ahead with a feature branch. > > One thing I had raised in the past was the desire to have a flag that would > generate machine readable output for nodetool commands. If this can be done > with a minor incremental effort, it would definitely reduce the burden on > operators / integrations that rely on the nodetool output. As I have earlier > indicated in the past, relying on human readable output for CLI tools like > nodetool is fragile and providing a JSON output as an alternative is a great > first step in eliminating that dependency. I'm just curious about the level > of effort. If it is too much or too invasive, we can consider producing JSON > output for inclusion in the next major release. > > On Fri, Jun 28, 2024 at 6:47 AM Maxim Muzafarov <mmu...@apache.org> wrote: > > > Hello everyone, > > > The nodetool relies on the airlift/airline library to mark up the CLI > commands used to manage Cassandra, which are part of our public API. > This library is no longer maintained, so we need to update it anyway, > and the good news is that we already have several good alternatives: > airline-2 [3] or picocli [2]. > > In this message, I'm mainly talking about CASSANDRA-17445 [4], which > refers to the problem and is a prerequisite for a larger CEP-38 CQL > Management API [5]. It doesn't make sense to use annotations from the > deprecated library to build a new API, so this is another reason to > update the library as soon as possible and do some inherently small > code refactoring required for the CEP-38. > > In addition to being widely used and well supported, the Picocli > library offers the following advantages for us: > - We can detach the jmx-specific parameters from the commands so that > they can be reused in other APIs (e.g. without host, port) while > remaining backwards compatible; > - We can set up nodetool's autocompletion after the migration with > minimal effort; > - There is a good Picocli ecosystem of tools that we can use to > simplify our codebase, e.g. generate man pages tool to make our CLIs > more Unix friendly [7]; > > > = Prototype = > > I have a working prototype [8] that shows what the result will look > like. The prototype includes: > - Tests between the execution of commands via the nodetool and nodtoolv2; > - 5 out of 164 nodetool commands have been moved so far, to show the > refactoring we need to do to the command's body; > - The command help output under for the nodetoolv2 is the same as it > is currently for the nodetool and this is the default, however a > "cassandra.cli.picocli.layout" is added to switch to the Picocli > defaults; > - You can also see that the colour scheme is applied by the Picocli > out of the box, and this is how it looks [9]; > - The nodetoolv2 is called first when the shell is triggered, and if > the nodetoolv2 doesn't contain the command it needs yet, it falls back > to the nodetool and the old argument parser; > > > Since the number of commands is quite large (164), I'd like to create > a feature branch and move all the commands one at a time, while > keeping the output backwards by applying additional tests at the same > time and checking that the CI is always green. I think the "feature > branch" approach will be less stressful for us since it focuses on > requiring a review of only tedious changes to the feature branch, > rather than reviewing the 15k line patch. > > > Anyway, I am open to any suggestions and advice based on your > experience and best practices for this case. Looking forward to your > thoughts and suggestions. > > > > [1] https://github.com/airlift/airline > [2] https://picocli.info/ > [3] https://github.com/rvesse/airline > [4] https://issues.apache.org/jira/browse/CASSANDRA-17445 > [5] > https://cwiki.apache.org/confluence/display/CASSANDRA/CEP-38%3A+CQL+Management+API > [6] > https://github.com/apache/cassandra/pull/2497/files#diff-acdd5f29d28df5c02f4bfc933528f084508b4923112e312e68a4aff7df973bce > [7] https://picocli.info/man/gen-manpage.html > [8] https://github.com/apache/cassandra/pull/2497/files > [9] > https://github.com/apache/cassandra/assets/3415046/57b14ae0-ff59-43d2-b542-10d3218ae075 > >
