On Thu, May 10, 2018 at 8:48 PM, Dan Smith <d...@danplanet.com> wrote:
The oslo UUIDField emits a warning if the string used as a field
value
does not pass the validation of the uuid.UUID(str(value)) call
[3]. All the offending places are fixed in nova except the
nova-manage
cell_v2 map_instances call [1][2]. That call uses markers in the DB
that are not valid UUIDs.
No, that call uses markers in the DB that don't fit the canonical
string
representation of a UUID that the oslo library is looking for. There
are
many ways to serialize a UUID:
https://en.wikipedia.org/wiki/Universally_unique_identifier#Format
The 8-4-4-4-12 format is one of them (and the most popular). Changing
the dashes to spaces does not make it not a UUID, it makes it not the
same _string_ and it's done (for better or worse) in the
aforementioned
code to skirt the database's UUID-ignorant _string_ uniqueness
constraint.
You are right, this is oslo specific. I think this weakens the severity
of the warning in this particular case.
If we could fix this last offender then we could merge the patch [4]
that changes the this warning to an exception in the nova tests to
avoid such future rule violations.
However I'm not sure it is easy to fix. Replacing
'INSTANCE_MIGRATION_MARKER' at [1] to
'00000000-0000-0000-0000-00000000' might work
The project_id field on the object is not a UUIDField, nor is it 36
characters in the database schema. It can't be because project ids are
not guaranteed to be UUIDs.
Correct. My bad. Then this does not cause any UUID warning.
but I don't know what to do with instance_uuid.replace(' ', '-') [2]
to make it a valid uuid. Also I think that if there is an unfinished
mapping in the deployment and then the marker is changed in the code
that leads to inconsistencies.
IMHO, it would be bad to do anything that breaks people in the middle
of
a mapping procedure. While I understand the desire to have fewer
spurious warnings in the test runs, I feel like doing anything to
impact
the UX or performance of runtime code to make the unit test output
cleaner is a bad idea.
Thanks for confirming my original bad feelings about these kind of
solutions.
I'm open to any suggestions.
We already store values in this field that are not 8-4-4-4-12, and the
oslo field warning is just a warning. If people feel like we need to
do
something, I propose we just do this:
https://review.openstack.org/#/c/567669/
It is one of those "we normally wouldn't do this with object schemas,
but we know this is okay" sort of situations.
Personally, I'd just make the offending tests shut up about the
warning
and move on, but I'm also okay with the above solution if people
prefer.
I think that was Takashi's first suggestion as well. As in this
particular case the value stored in the field is still a UUID just not
in the canonical format I think it is reasonable to silence the warning
for these 3 tests.
Thanks,
gibi
__________________________________________________________________________
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
