One thing that might be worth to keep in mind is that having a * as parameter might become annoying when people try to use it in a cli context with bash auto-expanding it.
Just saying :) It's overall a good idea though, but I'm not sure if implementing extra functionality with contextualized glob expansion is worth the effort. Do you think you or anyone else might have a recurrent use case for this that might warrant its introduction? Cheers, Morg. On 2 December 2016 at 15:10, 'Brian Foley' via ganeti-devel < ganeti-devel@googlegroups.com> wrote: > On Fri, Dec 02, 2016 at 04:53:51PM +0200, Yiannis Tsiouris wrote: > > On 12/02, Brian Foley wrote: > > > On Thu, Dec 01, 2016 at 12:34:11PM +0200, Yiannis Tsiouris wrote: > > > > This patch extends 'gnt-instance modify' by allowing a user to > remove a > > > > list of public/private OS parameters from an instance. This can be > > > > useful before performing a reinstall to a new OS provider. Example > > > > usage: > > > > > > > > $ gnt-instance modify --remove-os-parameters parm1,parm2 > <instance_name> > > > > > > > > or > > > > > > > > $ gnt-instance modify --remove-os-parameters-private parm3 > <instance_name> > > > > > > > > Signed-off-by: Yiannis Tsiouris <ts...@grnet.gr> > > > > > > Mostly LGTM, see a couple of inline comments. > > > > I agree with everything. > > > > > Also, a thought: I wonder could we merge the --clear-os-parameters > functionality > > > into --remove-os-parameters by having a special '*' parameter. If it's > the only > > > one present in the list, it means remove everything. It would mean we > could get > > > rid of all the boilerplate associated with parsing (and serialising) 2 > extra > > > options. > > > > Hmm, that's an interesting point. I think this could be more clear both > for the > > user and the code. I could do that! Just to be clear: I should treat the > string > > '*' as a "special" OS parameter; this doesn't mean that I should try to > globe > > based on '*' i.e. 'param*' doesn't expand to 'param1', 'param2', etc., > right? > > Good point. Just having literal "*" as a special parameter is the > simplest, but > I can imagine globbing, or perhaps even full regexp matching might well be > useful too. > > We can't just use Python's glob.glob because that operates on directories > only, > but we could make a variant of utils.text.DnsNameGlobPattern to match with > regexps for us. > > Cheers, > Brian. >
