[openstack-dev] [cinder] Custom fields for versioned objects

Ryan Rossiter rlrossit at linux.vnet.ibm.com
Tue Dec 15 15:08:57 UTC 2015


Thanks for the review Michal! As for the bp/bug report, there’s four options:

1. Tack the work on as part of bp cinder-objects
2. Make a new blueprint (bp cinder—object-fields)
3. Open a bug to handle all changes for enums/fields
4. Open a bug for each changed enum/field

Personally, I’m partial to #1, but #2 is better if you want to track this work separately from the other objects work. I don’t think we should go with bug reports because #3 will be a lot of Partial-Bug and #4 will be kinda spammy. I don’t know what the spec process is in Cinder compared to Nova, but this is nowhere near enough work to be spec-worthy.

If this is something you or others think should be discussed in a meeting, I can tack it on to the agenda for tomorrow.

> On Dec 15, 2015, at 3:52 AM, Michał Dulko <michal.dulko at intel.com> wrote:
> 
> On 12/14/2015 03:59 PM, Ryan Rossiter wrote:
>> Hi everyone,
>> 
>> I have a change submitted that lays the groundwork for using custom enums and fields that are used by versioned objects [1]. These custom fields allow for verification on a set of valid values, which prevents the field from being mistakenly set to something invalid. These custom fields are best suited for StringFields that are only assigned certain exact strings (such as a status, format, or type). Some examples for Nova: PciDevice.status, ImageMetaProps.hw_scsi_model, and BlockDeviceMapping.source_type.
>> 
>> These new enums (that are consumed by the fields) are also great for centralizing constants for hard-coded strings throughout the code. For example (using [1]):
>> 
>> Instead of
>>    if backup.status == ‘creating’:
>>        <do_stuff>
>> 
>> We now have
>>    if backup.status == fields.BackupStatus.CREATING:
>>        <do_stuff>
>> 
>> Granted, this causes a lot of brainless line changes that make for a lot of +/-, but it centralizes a lot. In changes like this, I hope I found all of the occurrences of the different backup statuses, but GitHub search and grep can only do so much. If it turns out this gets in and I missed a string or two, it’s not the end of the world, just push up a follow-up patch to fix up the missed strings. That part of the review is not affected in any way by the RPC/object versioning.
>> 
>> Speaking of object versioning, notice in cinder/objects/backup.py the version was updated to appropriate the new field type. The underlying data passed over RPC has not changed, but this is done for compatibility with older versions that may not have obeyed the set of valid values.
>> 
>> [1] https://review.openstack.org/#/c/256737/
>> 
>> 
>> -----
>> Thanks,
>> 
>> Ryan Rossiter (rlrossit)
> 
> Thanks for starting this work with formalizing the statuses, I've
> commented on the review with a few remarks.
> 
> I think we should start a blueprint or bugreport to be able track these
> efforts.
> 
> 
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


-----
Thanks,

Ryan Rossiter (rlrossit)




More information about the OpenStack-dev mailing list