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.du...@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-requ...@lists.openstack.org?subject:unsubscribe > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ----- Thanks, Ryan Rossiter (rlrossit) __________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
