I agree, having a DISCUSS thread with a specific subject line is less likely to be overlooked.
One thing I'd like to note here is PicoCLI and Airline 2 are independent projects that are ALv2 licensed. A subset of the Cassandra contributors may have difficulty contributing to such projects due to preexisting policies that their employers may have in place. I am concerned about hostile licensing changes in the future which will necessitate another migration for us. That said, is there a specific reason to not consider Apache Commons CLI[1]? Dinesh [1] https://commons.apache.org/proper/commons-cli/ On Mon, Jul 8, 2024 at 10:22 AM David Capwell <dcapw...@apple.com> wrote: > I don't think that a separate thread would add extra visibility > > > Disagree. This thread is about adding a feature branch, so many could > ignore if they don’t care. The fact you are switching the library (and > which one) is something we have to hunt for. By having a new DISCUSS > thread it makes it clear which library you wish to add, and people can sign > off if they care or not. > > I wouldn’t create this thread until you settle on which one you wish to > move forward with. > > Is adding the PicoCLI library as a project dependency getting any objections > from the Community? > > > Thats the point of the new DISCUSS thread. By being very clear you wish > to add PicoCLI people can either validate we are allowed to, or raise any > objections. I have not really seen any pushback so far outside of 1 case > that wasn’t legally allowed to be used. > > Take a look at previous threads about adding different libraries. > > On Jul 8, 2024, at 7:58 AM, Caleb Rackliffe <calebrackli...@gmail.com> > wrote: > > +1 on picocli > > RE the feature branch, I would just maintain the feature branch in your > own fork to break out whatever "reviewable units" of code you want. When > all the incremental review is done (I have no problem going back and > forth), squash everything together, do whatever additional testing you > need, and commit. > > On Fri, Jul 5, 2024 at 10:40 AM Maxim Muzafarov <mmu...@apache.org> wrote: > >> > 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 >> > >> > >> > >
