daniellavoie edited a comment on issue #5580:
URL:
https://github.com/apache/incubator-pinot/issues/5580#issuecomment-646921816
Definitely an upgrade! This is a lot more consistent to the experience you
get in the go / node cli world.
### Foreground or background
I feel the CLI should start the service in foreground by default unless
specified otherwise. As such, only the `stop` command should only be accepted
when a command like `start-background` was used. Starting in foreground sounds
more natural for Docker and Kubernetes deployments. Also, if you are starting
the CLI as a developper, you get immediate output in your terminal without
looking for log files anywhere.
### Feeding properties from multiple sources.
As per my previous comment, I'm working on supporting an abstraction that
decouples the property sources from the code base. This is a heavy refactoring
that introduces a wrapper around the
`org.apache.commons.configuration.Configuration` used all over Pinot. This
wrapper introduces a concept of `CompositeConfiguration` that can accept
variables from CLI args, environment variables and property files specified by
`config.paths`. This new wrapper also introduces a concept of relaxed binding.
What it means is that it knows how to translate environment variables such as
`PINOT_CONTROLER_PORT` to properties `controller.port`. When properties are
inserted and fetched from the environment, they go through relaxing routine
that standardize their key so that lookup such
as `config.getPropert("controller.port")` return the value even if it was
defined by the `PINOT_CONTROLER_PORT` env variable . As I exposed in my
previous comment, this is intended to offer a transparent mechanism to drive
Pinot configuration without requiring config files that are hard to reference
in a deployment manifest without volumes.
### Passing Pinot properties to the CLI.
Given the new CLI. I would think it would be nice if any property loaded in
`org.apache.commons.configuration.Configuration` could be passed as argument in
the startup command, e.g.:
```
$ pinot start controller --help
Usage:
pinot start controller [options]
Available Options:
--help, -h
--debug
--controler.port=<port>
--controler.host=<hostname>
--cluster-name=<cluster-name>
--[etc]
...
Refer to Pinot Documentation at
https://docs.pinot.apache.org/basics/components/controller#starting-a-controller
for full reference of all available properties for the Controller.
```
Now, if we want to allow any Pinot config properties to be passed through a
`pinot` cli, I feel we might have to drop `args4j` since it requires any CLI
argument to be explicitly annotated in a `SubCommand` object. I think overall,
Pinot has more than 100 properties used all over the place and does not feel
like it would be maintainable over time to require manual binding between a
`SubCommand` class and the `org.apache.commons.configuration.Configuration`
class. Rewriting a custom CLI with his own main startup class would grant
access to the main `args` and let us parse all properties and inject them in
the new `CompositeConfiguration` to merge them with the env variables and
explicit config files.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
