Re: [openstack-dev] [Nova] object field naming question
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 mailto: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)" mailto: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.org<mailto: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
> 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
Re: [openstack-dev] [Nova] object field naming question
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
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
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
[openstack-dev] [Nova] object field naming question
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). 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. 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)? 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