Re: [openstack-dev] [Nova] object field naming question

2014-10-10 Thread Murray, Paul (HP Cloud)
I think the hyphen question is settled – we can go with hv_type and vm_mode to 
be in line with the rest of nova.

Now for the other bit…

After this email thread and discussion in the IRC with those who expressed 
interest I have come to the following:

I will use the name supported_hv_specs instead of supported_instances in the 
ComputeNode object and HVSpec for the new object. So the field type of 
supported_hv_specs will be ListOfObjectsField(‘HVSpec’).

The supported_instances field in the compute_nodes table will be left as it is. 
This means that supported_hv_specs in ComputeNodes will map to the 
supported_instances field in the compute_nodes table.

Thanks for helping,
Paul




From: Paul Murray [mailto:ptma...@gmail.com]
Sent: 10 October 2014 13:00
To: Murray, Paul (HP Cloud)
Subject: Fwd: [openstack-dev] [Nova] object field naming question


-- Forwarded message --
From: Dan Smith d...@danplanet.commailto:d...@danplanet.com
Date: 9 October 2014 17:40
Subject: Re: [openstack-dev] [Nova] object field naming question
To: OpenStack Development Mailing List (not for usage questions) 
openstack-dev@lists.openstack.orgmailto:openstack-dev@lists.openstack.org


 The value it adds (and that an underscore would add in hvtype -
 hv_type) is that the name would match the naming style for the vast
 majority of everything else in OpenStack. See, for examples:

Agreed.

 As mentioned in the review, I disagree on this point, since doing a
 cleanup afterwards would mean having to increment the
 nova.objects.SupportedInstance model VERSION as soon as it went into
 master. I say let's make the quick change now and avoid having to write
 code like this in the next patchset:

  if client_version = (1,0):
 # We renamed hvtype to hv_type in 1.1
 self.hv_type = fields.get('hvtype')

Right, this becomes RPC debt if we think we might change it later. We
definitely want to get it right the first time whenever possible.

--Dan


___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.orgmailto:OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [Nova] object field naming question

2014-10-09 Thread Daniel P. Berrange
On Thu, Oct 09, 2014 at 01:45:43PM +, Murray, Paul (HP Cloud) wrote:
 Hi All,
 
 The following question relates to this change:
 
 https://review.openstack.org/#/c/125091/
 
 This change adds a field to the ComputeNode object called 
 supported_instances. It also adds an object called SupportedInstance that 
 has fields called
 arch, hvtype and vm_mode.
 
 All these names already exist in the nova code, but when put together in
 these objects they seem a little odd (i.e. supported_instances may be a
 little misleading, hvtype has no hyphen but vm_mode does). This is where
 they come from:
 
 -  supported_instances is the name of the corresponding field of
  the compute_nodes database table. The supported_instances field actually
  contains a the list of architecture, hypervisor type and vm_mode
  combinations supported by the compute node. It is also the existing
  field name used in a dict provided by the virt drivers to report this
  list to nova.
 
 
 -  arch, hvtype and vm_mode are the names used by variables
   throughout nova that refer to the relevant data obtained contained
   in supported_instances.
 
 The question is: are these the names we actually want to use?

I'd like to just kill the underscore in 'vm_mode' as I don't think it
adds any real value.

 Let me know opinions and I will fix the patch accordingly.

I don't think we want to block your patch on this item. We can just do a
cleanup afterwards, rather than mixing it in with your patch.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [Nova] object field naming question

2014-10-09 Thread Jay Pipes

On 10/09/2014 10:16 AM, Daniel P. Berrange wrote:

On Thu, Oct 09, 2014 at 01:45:43PM +, Murray, Paul (HP Cloud) wrote:

Hi All,

The following question relates to this change:

https://review.openstack.org/#/c/125091/

This change adds a field to the ComputeNode object called supported_instances. It also 
adds an object called SupportedInstance that has fields called
arch, hvtype and vm_mode.

All these names already exist in the nova code, but when put together in
these objects they seem a little odd (i.e. supported_instances may be a
little misleading, hvtype has no hyphen but vm_mode does). This is where
they come from:

-  supported_instances is the name of the corresponding field of
  the compute_nodes database table. The supported_instances field actually
  contains a the list of architecture, hypervisor type and vm_mode
  combinations supported by the compute node. It is also the existing
  field name used in a dict provided by the virt drivers to report this
  list to nova.


-  arch, hvtype and vm_mode are the names used by variables
   throughout nova that refer to the relevant data obtained contained
   in supported_instances.

The question is: are these the names we actually want to use?


I'd like to just kill the underscore in 'vm_mode' as I don't think it
adds any real value.


The value it adds (and that an underscore would add in hvtype - 
hv_type) is that the name would match the naming style for the vast 
majority of everything else in OpenStack. See, for examples:


 We use vm_state not vmstate. power_state, not powerstate, port_status 
not portstatus, disk_container not diskcontainer, etc.



Let me know opinions and I will fix the patch accordingly.


I don't think we want to block your patch on this item. We can just do a
cleanup afterwards, rather than mixing it in with your patch.


As mentioned in the review, I disagree on this point, since doing a 
cleanup afterwards would mean having to increment the 
nova.objects.SupportedInstance model VERSION as soon as it went into 
master. I say let's make the quick change now and avoid having to write 
code like this in the next patchset:


 if client_version = (1,0):
# We renamed hvtype to hv_type in 1.1
self.hv_type = fields.get('hvtype')

Best,
-jay

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [Nova] object field naming question

2014-10-09 Thread Jay Pipes

On 10/09/2014 09:45 AM, Murray, Paul (HP Cloud) wrote:

Hi All,

The following question relates to this change:

https://review.openstack.org/#/c/125091/

This change adds a field to the ComputeNode object called
“supported_instances”. It also adds an object called “SupportedInstance”
that has fields called “arch”, “hvtype” and “vm_mode”.

All these names already exist in the nova code, but when put together in
these objects they seem a little odd (i.e. supported_instances may be a
little misleading, hvtype has no hyphen but vm_mode does). This is where
they come from:

-supported_instances is the name of the corresponding field of the
compute_nodes database table. The supported_instances field actually
contains a the list of architecture, hypervisor type and vm_mode
combinations supported by the compute node. It is also the existing
field name used in a dict provided by the virt drivers to report this
list to nova.

-arch, hvtype and vm_mode are the names used by variables throughout
nova that refer to the relevant data obtained contained in
supported_instances.

The question is: are these the names we actually want to use?

The reason I ask is that once the names are used for Nova object fields,
the nova object versions would need to be bumped to change them. So if
they are going to change, now would be a good time to put the right
thing in the objects (the rest of nova could be changed subsequently).


Right, exactly.


So, do you think supported_instances should be something else, e.g.
supported_hv_properties? (The SupportedInstance object name can follow
this one.) Please make a suggestion if you think it should change.


Yeah, supported_hv_properties works for me. ++


Do you think we should have hyphens or not in hvtype and vm_mode (note,
vm_mode occurs 160+ times in nova and vmmode occurs 12 times, whereas
occurrence of hv_type vs hvtype has the opposite bias).

In fact, is there a convention that should be followed that I have
forgotten about (or never knew)?


The convention throughout OpenStack code is to use underscore 
separation. Not having underscore separation is the exception, not the rule.


Best,
-jay


Let me know opinions and I will fix the patch accordingly.

Thanks,

Paul



___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev



___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [Nova] object field naming question

2014-10-09 Thread Dan Smith
 The value it adds (and that an underscore would add in hvtype -
 hv_type) is that the name would match the naming style for the vast
 majority of everything else in OpenStack. See, for examples:

Agreed.

 As mentioned in the review, I disagree on this point, since doing a
 cleanup afterwards would mean having to increment the
 nova.objects.SupportedInstance model VERSION as soon as it went into
 master. I say let's make the quick change now and avoid having to write
 code like this in the next patchset:
 
  if client_version = (1,0):
 # We renamed hvtype to hv_type in 1.1
 self.hv_type = fields.get('hvtype')

Right, this becomes RPC debt if we think we might change it later. We
definitely want to get it right the first time whenever possible.

--Dan



signature.asc
Description: OpenPGP digital signature
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev