Bug 890294

Summary: [RFE] ovirt-engine-cli: Some objects fields names are not clear and need to rephrase
Product: Red Hat Enterprise Virtualization Manager Reporter: Oded Ramraz <oramraz>
Component: ovirt-engine-cliAssignee: Juan Hernández <juan.hernandez>
Status: CLOSED WONTFIX QA Contact: Shai Revivo <srevivo>
Severity: high Docs Contact:
Priority: unspecified    
Version: 3.2.0CC: acathrow, ecohen, iheim, jkt, juan.hernandez, oramraz, Rhev-m-bugs, srevivo
Target Milestone: ---Keywords: FutureFeature, Improvement
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard: infra
Fixed In Version: Doc Type: Enhancement
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2014-06-16 15:28:36 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: Infra RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Oded Ramraz 2012-12-26 08:47:17 UTC
Description of problem:

Some objects fields names are not clear and need to rephrase:

Few examples ( out of many ):

Host object: power_management-options-option-name , power_management-options-option-value ,storage_manager-valueOf
Cluster: memory_policy-transparent_hugepages-enabled , error_handling-on_error
VM: placement_policy-affinity 
Statistic :  values-value-datum
Datacenter: status-state , supported_versions-version-major,minor ...


The proper solution to this problem might be to add translation layer instead of using names which derives from Rest API and the xml tags names. 

Version-Release number of selected component (if applicable):


How reproducible:


Steps to Reproduce:
1.
2.
3.
  
Actual results:


Expected results:


Additional info:

Comment 1 Michael Pasternak 2012-12-26 09:08:12 UTC
(In reply to comment #0)
> Description of problem:
> 
> Some objects fields names are not clear and need to rephrase:
> 
> Few examples ( out of many ):
> 
> Host object: power_management-options-option-name ,
> power_management-options-option-value ,storage_manager-valueOf

> Cluster: memory_policy-transparent_hugepages-enabled ,
> error_handling-on_error
> VM: placement_policy-affinity 
> Statistic :  values-value-datum
> Datacenter: status-state , supported_versions-version-major,minor ...
> 
> 
> The proper solution to this problem might be to add translation layer
> instead of using names which derives from Rest API and the xml tags names. 
> 

Oded,

All names mentioned above (except of storage_manager-valueOf which is python assumption) is naming conventions decided on upstream by community, - i can't
see any reason for breaking this convention, especially that it would break backward compatibility.

Comment 2 Oded Ramraz 2012-12-26 09:16:12 UTC
(In reply to comment #1)
> (In reply to comment #0)
> > Description of problem:
> > 
> > Some objects fields names are not clear and need to rephrase:
> > 
> > Few examples ( out of many ):
> > 
> > Host object: power_management-options-option-name ,
> > power_management-options-option-value ,storage_manager-valueOf
> 
> > Cluster: memory_policy-transparent_hugepages-enabled ,
> > error_handling-on_error
> > VM: placement_policy-affinity 
> > Statistic :  values-value-datum
> > Datacenter: status-state , supported_versions-version-major,minor ...
> > 
> > 
> > The proper solution to this problem might be to add translation layer
> > instead of using names which derives from Rest API and the xml tags names. 
> > 
> 
> Oded,
> 
> All names mentioned above (except of storage_manager-valueOf which is python
> assumption) is naming conventions decided on upstream by community, - i can't
> see any reason for breaking this convention, especially that it would break
> backward compatibility.

I didn't meant to rephrase those names in Rest API , I just meant to display them differently in CLI :
For example : 

<values type="INTEGER"><value><datum>16712204288</datum></value>  looks fine in Rest , but values-value-datum is not clear name for CLI . 

<memory_policy><overcommit percent="100"/><transparent_hugepages><enabled>true</enabled> looks fine from Rest but memory_policy-transparent_hugepages-enabled is too long / unclear field name and need to rephrase .

Comment 3 Michael Pasternak 2012-12-26 09:27:34 UTC
> I didn't meant to rephrase those names in Rest API , I just meant to display
> them differently in CLI :
> For example : 
> 
> <values type="INTEGER"><value><datum>16712204288</datum></value>  looks fine
> in Rest , but values-value-datum is not clear name for CLI . 
> 
> <memory_policy><overcommit
> percent="100"/><transparent_hugepages><enabled>true</enabled> looks fine
> from Rest but memory_policy-transparent_hugepages-enabled is too long /
> unclear field name and need to rephrase .

i understand this, but:

1. you forgetting that cli is out in 3.1, changing will mean breaking backward compatibility

2. making names shorter is not practical/doable because if option looks like
--x-y-z-q, leaving only --q can become not unique in the object cause same q
element can reside in the y and z levels, take 'status-state' for instance,
'state' element can appear in the different levels of the same object.

3. i don't think that showing --transparent_hugepages-enabled is better then
--memory_policy-transparent_hugepages-enabled, i think it's more informative especially when you don't have to type the option name manually,

i.e making <TAB><TAB> on --memory_policy will show all available options for the  memory_policy, e.g:

[oVirt shell (connected)]# add cluster memory_policy-
memory_policy-overcommit-percent             
memory_policy-transparent_hugepages-enabled  
....

what is cannot be achieved when you have single period option.

Comment 4 Simon Grinberg 2012-12-26 14:06:02 UTC
(In reply to comment #3)
> i understand this, but:
> 
> 1. you forgetting that cli is out in 3.1, changing will mean breaking
> backward compatibility
> 

Michael, 
This discussion keeps on coming up every time we encounter these long name.

I also agree that an automatic rule for making these shorter may be a problem when this maps to the same name. 

How about using an aliases file.
Yes, manually maintained file - we suggest the defaults mapping (QE will be happy to assist), while the customer may be allowed to override these mappings.

This way:
1. Backwards compatibility is maintained, same as parity with SDK and API
2. We make it more human friendly, up to the specific human using the CLI

The way it works is: 
Tab also shows aliases (in parentheses). 
User may either use alias or the original

Thoughts?

Comment 5 Michael Pasternak 2012-12-26 14:30:37 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > i understand this, but:
> > 
> > 1. you forgetting that cli is out in 3.1, changing will mean breaking
> > backward compatibility
> > 
> 
> Michael, 
> This discussion keeps on coming up every time we encounter these long name.

by very same person :)

> 
> I also agree that an automatic rule for making these shorter may be a
> problem when this maps to the same name. 
> 
> How about using an aliases file.
> Yes, manually maintained file - we suggest the defaults mapping (QE will be
> happy to assist), while the customer may be allowed to override these
> mappings.
> 
> This way:
> 1. Backwards compatibility is maintained, same as parity with SDK and API

then we earned nothing, from my experience things that are not enforced - not get adapted.

> 2. We make it more human friendly, up to the specific human using the CLI

i can't see how possibly you can make human-friendly/shorter something like
memory_policy-transparent_hugepages-enabled, th-enabled ? how would someone know
that it's related to the memory_policy? new names will be awkward and disconnected from the context/api/sdk/UI, in my view new terminology will only make cli more complex.

> 
> The way it works is: 
> Tab also shows aliases (in parentheses). 
> User may either use alias or the original
> 
> Thoughts?

i'm not sure Simon, 

1) i made my point in Comment 3, section 3., i think having fully expended option names gives user ability to group similar options on <TAB>, this will be broken using aliases and will work only in the alphabetic manner and not logical-relation, plus aliases won't be informative as current representation.

2) don't forget that at the moment options and actual object's properties has same naming convention [1], so they has to be rephrased as well? ...

[1]

[oVirt shell (connected)]# add cluster memory_policy-
memory_policy-overcommit-percent             
memory_policy-transparent_hugepages-enabled 

[oVirt shell (connected)]# show cluster Default

id                                         : 99408929-82cf-4dc7-a532-9d998063fa95
name                                       : Default
error_handling-on_error                    : migrate
gluster_service                            : False
memory_policy-overcommit-percent           : 100
memory_policy-transparent_hugepages-enabled: True
version-major                              : 3
version-minor                              : 0
virt_service                               : True

Comment 6 Simon Grinberg 2012-12-27 14:47:05 UTC
> i can't see how possibly you can make human-friendly/shorter something like
> memory_policy-transparent_hugepages-enabled, th-enabled ? how would someone
> know
> that it's related to the memory_policy? new names will be awkward and
> disconnected from the context/api/sdk/UI, in my view new terminology will
> only make cli more complex.

But these are already different from the terminology in the GUI, and thus sometimes confusing.

My problem is:
API and SDK users are one class.
GUI users are another

CLI users will be closer to the GUI users then the SDK users, thus sticking to the SDK convention is actually more confusing to them.

Example:
In the GUI there is only memory-policy, and you have only 3 options, off, Desktops, Servers.

In the CLI it will look like: 
memory_policy-overcommit-percent           : 100/150/200

And the Huge pages is always on
memory_policy-transparent_hugepages-enabled: True

Now we need to bridge the gap somehow. And yes I'm aware that this requires considerable amount of work.

Comment 9 Itamar Heim 2014-06-16 15:28:36 UTC
Closing old bugs. If this issue is still relevant/important in current version, please re-open the bug.