On Tue, Jan 03, 2017 at 09:35:20AM +0000, Federico Pareschi wrote: > Interesting, you raise valid points for a patch that I thought was > trivial but indeed is not. > > > Per our discussion yesterday, all the gnt-group commands actually do work > > with > > either group specified by name or by UUID, so this is an improvement to the > > docs. However, the command line help in lib/client/gnt_group.py also uses > > the > > string "group-name". Could you update these too please? > > > Yes, good point. > > > Similarly, could you make sure you update all the other lib/client/*.py CLI > > arg > > strings to match the changes you've made? > > > Most of the other commands, surprisingly, were already correct in > their .py files, > however I have fixed the few that were not.
Great. Thanks. > > Interestingly, gnt-instance shutdown/startup work correctly with either > > UUIDs, > > or prefix matches on instance FQDN. However, strangely, some of the > > subcommands > > like gnt-instance info will do prefix match on instance FQDN, but don't work > > with UUID, so I think we can't just bindly change these without checking > > what > > each subcommand actually uses. > > > This looks like a symptom of something broken/inconsistent that should > get fixed. > First of all, the changes I had made were already in the .py helper > strings, interestingly enough. i.e. gnt-instance reboot --help already > shows <instance> and not <name> so I think that's a good change as is > (and indeed it does work with UUIDs as well). > > The real problem, though, is that as you said some of these commands > are just inconsistent. For example all the instance tagging commands > do not work with UUIDs, however the group tagging does work with UUIDs > and they all use the same base code. This leads me to believe there is > some inconsistency in the backend code for tagging objects through > UUIDs vs the object name and it should get fixed, but it's probably > low priority and outside of the scope of this patch. > > Now the question is, do we just avoid touching all the tags-related > info strings and fix the rest of the man pages as proposed in this > patch, or do we change the tags-string for group (but not for > instance/node objects) leaving them inconsistent in the documentation > (but consistent in the way the code behaves)? Good question. Before I went on holidays I tried to figure out where exactly the arg parsing/'name resolution' was being done, and as you say it seems inconsistent. My inclination would be to say that it's better for the docs to be accurate and expose the inconsistency in the implementation, than to have nice 'regular' docs that aren't accurate. If we intend to fix the inconsistent implementation in the near future, I'd say leave the docs alone, and just fix the implementation; but if we don't want to go down the rabbit-hole of fixing the impl, then just update the docs. [snip] > > gnt-node list-drbd, list-tags definitely looks up by prefix match on FQDN or > > UUID. Is this definitely true for gnt-node add, and the others though? > > I tried one example, gnt-node remove <UUID> and indeed it does not > work with UUIDs. This really bothers me as leaving it as it is, with > node_name, makes it inconsistent with a lot of the rest of the code > where UUIDs do work. Again, we should be fixing this but it's probably > very low priority compared to other issues. Agreed. It's pretty low priority, and I don't recall anyone else complaining about it. > I'll revert the changes in those flags that do not use UUIDs. I'll > send a new patch after we've decided what we should do with the > tags-related code as I mentioned earlier. Thanks, Brian.
