Hi Dawid, For the first comment, I am also up for re-using as many options as possible. My only concern though is that so far the -p and the parallelism.default were considered "different" and I cannot tell the exact reason. In addition, the "parallelism.default" is not the most descriptive name in my opinion. So just for caution, I would go with introducing a new option and if we all agree, I would actually deprecate the old one, as that is the one with the problematic name.
For the second comment, you are right for the dynamic properties and I will remove them from the document. For the rest (i.e. the "shutdown after exit" and the "attached"), I do not think we can remove them as the user should be allowed to specify them in the configuration. I am hoping to have more opinions on this, and I will open a voting thread as soon as we reach a consensus. Cheers, Kostas On Tue, Oct 22, 2019 at 10:49 AM Dawid Wysakowicz <[email protected]> wrote: > > Hi, > > I really like the idea of unifying all the ways to set a configuration > option and its keys. I think it nicely aligns with FLIP-59. > > I have two comments though. > > 1. I wonder what are you thoughts on reusing the "parallelism.default" > (CoreOptions.DEFAULT_PARALLELISM) instead of introducing a new key > "execution.parallelism". I am not sure if we should introduce another > key as it might be confusing for users to have two separate config > options for client and cluster side. > > Moreover if we go with a new config option for the client side with a > default value of 1, it makes the cluster side option unusable. Right now > the logic is that if we set the parallelism to -1(the default value) on > the client side, the cluster side configuration is used. > > Would be interested in an opinion of others on that matter. I think we > have few other cases of options that we set on the client side with a > fallback on the cluster side. I think we should have a common approach > to such scenarios. > > 2. If I understood it correctly the "dynamic-properties" (maybe also the > "attached" and "shutdown-on-exit") are an internal options that should > never be set manually by the user through the config file. I think they > are internal and should not be exposed. I wonder if we can make them > less visible somehow. Also I think we do not need to agree on them > through the FLIP process. > > Best, > > Dawid > > On 22/10/2019 10:05, Aljoscha Krettek wrote: > > I think this is good to go to a VOTE. We should, however, make it more > > prominent that we don’t like the “attached”, “shutdown-on-attached-exit”, > > and “dynamic-properties” and that we only add them for backwards > > compatibility. > > > > Best, > > Aljoscha > > > >> On 21. Oct 2019, at 17:48, Kostas Kloudas <[email protected]> wrote: > >> > >> Hi all, > >> > >> As part of FLIP-73 (the Executors effort), we would like to introduce > >> some new configuration options. These are needed in order to be able > >> to map all the options that the user can specify either > >> programmatically or through the CLI into configuration options. > >> > >> The bylaws specify that every user-facing change should pass through a > >> FLIP, so the FLIP with more details on the new options can be found > >> here: > >> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=133631524 > >> > >> Please keep the discussion in the mailing list. > >> > >> Thanks, > >> Kostas >
