Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-06-18
13:33:47 opendevreview Balazs Gibizer proposed openstack/nova-specs master: Fix the bp link in the cyborg admin token spec https://review.opendev.org/c/openstack/nova-specs/+/795493
13:34:00 gibi sean-k-mooney: ^^
13:35:41 sean-k-mooney gibi++
13:35:53 sean-k-mooney we dont have a karma bot here but still
13:36:19 gibi sean-k-mooney: thanks
13:36:32 gibi the happy days of doing the virtual paperwork as a PTL
13:36:57 sean-k-mooney at least you dont have to do it in triplicate
13:37:48 sean-k-mooney not today but i can proably look at adding a small script that will check for this in the ci
13:38:12 gibi good idea
13:38:20 gibi on the CI script
13:38:40 artom lyarwood, done
13:38:41 sean-k-mooney get the added filename, strip the rst, grep and see if there is a url that end with that and curl it to make sure it does not have a 404
13:43:49 lyarwood thanks
13:45:48 stephenfin an interesting Python problem
13:45:51 stephenfin import json
13:45:51 stephenfin class Foo:
13:45:51 stephenfin @property
13:45:51 stephenfin def bar(self):
13:45:51 stephenfin return json.loads(self._bar)
13:45:53 stephenfin @bar.setter
13:45:54 stephenfin def bar(self, value):
13:45:56 stephenfin self._bar = json.dumps(value)
13:45:58 stephenfin foo = Foo()
13:46:02 stephenfin foo.bar = {}
13:46:04 stephenfin foo.bar['test'] = 'hello'
13:46:06 stephenfin assert foo.bar == {'test': 'hello'}
13:46:08 stephenfin ^ that fails
13:46:31 sean-k-mooney yes
13:46:32 sean-k-mooney that is expected
13:46:35 sean-k-mooney but you can fix that
13:46:37 stephenfin I get why (the setter is called for setting the attribute itself, not attributes of the attribute) but I don't know how to fix it
13:47:02 sean-k-mooney foo.bar is returing a copy of the data in self._bar
13:47:07 sean-k-mooney which is a dict
13:47:25 sean-k-mooney acn dyou cant do {} = {"key":"val"}
13:48:28 stephenfin well I need to fix it, because I've got a bug here https://github.com/openstack/nova/blob/master/nova/objects/migrate_data.py#L71-L89
13:48:51 stephenfin that's the pattern I used there and it doesn't work - the 'OS_VIF_DELEGATION' attribute of the embedded profile is never set :-(
13:49:24 sean-k-mooney i fixed this in one of my pathces
13:49:46 sean-k-mooney well for the fiels i added
13:51:29 stephenfin if you can find that I'd like to have a look at it, because right now I'm stumped
13:51:57 sean-k-mooney i tough tit was that to be honnest
13:51:57 jkulik foo.bar = foo.bar.update({'test': 'hello'}) ?
13:52:26 jkulik too far off from the initial intention?
13:52:36 sean-k-mooney jkulik: well the fix is to inste do the update on the underling dict
13:53:09 sean-k-mooney so yes that works
13:54:00 stephenfin dict.update doesn't return anything, so the idea is sound but it needs a slight tweak
13:54:34 sean-k-mooney right not update but you read modify write
13:54:42 gibi yepp the problem is that you have a getter and a setter but foo.bar['test'] only use the getter but not the setter :)
13:54:50 stephenfin foo.bar = dict(foo.bar, **{'test': 'hello'})
13:55:04 stephenfin will do the trick. Still not as pretty but at least it works
13:55:16 sean-k-mooney https://github.com/openstack/nova/blob/master/nova/objects/migrate_data.py#L81-L89 should work though
13:55:28 stephenfin gibi: Yeah. Slightly amazed that I haven't (that I can recall) hit this in all my years writing Python :D
13:55:46 sean-k-mooney i hit a proably with it and change it
13:55:52 stephenfin sean-k-mooney: the getter works, the setter does not
13:55:56 gibi I think we need get rid of the foo.bar['aaa'] by returning a frozen dict
13:55:56 stephenfin at least not in tests
13:56:28 sean-k-mooney stephenfin: its suoudl this was one of the things i had to workaround when writhing tha tpatch
13:56:31 gibi as the getter returns a copy of the underlining data not a reference for it
13:56:48 gibi so we have to tell the caller that it is copy
13:56:52 gibi not a live ref
13:56:55 jkulik frozen dict sounds good. then nobody can accidentally use the non-working pattern
13:57:49 sean-k-mooney frozen dict wont work
13:58:03 sean-k-mooney the issue is we are storing a json string
13:58:19 gibi sean-k-mooney: it does not solve the problem, it forces the caller to avoid modifying what is returned
13:58:23 stephenfin well it would indicate that you can't update it
13:58:25 sean-k-mooney and then we are returning it as a dict and we want update to that property to modify the json
13:58:37 sean-k-mooney gibi: right but we want them to be able to do that
13:58:38 stephenfin however, there's no such thing in stdlib
13:58:43 gibi sean-k-mooney: I don't :)
13:58:56 sean-k-mooney well for this code we are talkign too we do
13:59:10 sean-k-mooney other wise we shoudl jsut remove the property
13:59:21 sean-k-mooney and force use to work direclty on the json
13:59:43 opendevreview Merged openstack/nova-specs master: Fix the bp link in the cyborg admin token spec https://review.opendev.org/c/openstack/nova-specs/+/795493
13:59:45 sean-k-mooney the only reason the properties exist is to to give a dict like interface to the json blobs
14:00:00 sean-k-mooney stephenfin: what test is failing
14:00:08 gibi sean-k-mooney: then we need to make it one level deper allowing to set per key
14:00:24 gibi like foo.bar.test = 'aaa'
14:00:25 stephenfin none currently, because we don't have tests covering this code path
14:00:46 gibi sean-k-mooney: but that needs more work on the implementation side
14:01:16 sean-k-mooney stephenfin: the quick fix is as follows
14:01:18 sean-k-mooney self.profile[OS_VIF_DELEGATION] = supported
14:01:24 sean-k-mooney becomes
14:01:38 kashyap Am I hallucinating, or were the check marks of x, ✔, and ? used to be in colour on the support-matrix page? - https://docs.openstack.org/nova/wallaby/user/support-matrix.html
14:02:09 sean-k-mooney data = jsonutils.loads(self.profile_json); data[OS_VIF_DELEGATION]=supported; self.profile_json = jsonutils.dumps(data);
14:02:27 sean-k-mooney stephenfin: here https://github.com/openstack/nova/blob/master/nova/objects/migrate_data.py#L89
14:03:01 sean-k-mooney kashyap: they were yes
15:14:23 gibi stephenfin: could you please hit https://review.opendev.org/c/openstack/osc-placement/+/794276 when you have time
15:54:47 gmann stephenfin: lyarwood if that broken on fedora having py3.9 ? but we do have py3.9 job running successfully though those are n-v
15:56:32 gmann is that
16:22:21 opendevreview Merged openstack/nova master: Handle OPERATION_FAILED error during detach https://review.opendev.org/c/openstack/nova/+/796255
17:15:25 stephenfin lyarwood: Good thing you asked for that test. This code is doing nothing currently 😇
17:15:54 opendevreview Stephen Finucane proposed openstack/nova master: objects: Fix VIFMigrateData.supports_os_vif_delegation setter https://review.opendev.org/c/openstack/nova/+/797142
17:15:59 stephenfin sean-k-mooney: ^
17:16:24 stephenfin I haven't run those tests locally. I want to push it to the gate and see if the interfaces are correctly created in the Tempest job
17:16:53 stephenfin gmann: The main issue I was seeing is that the deps in lower-constraints don't work with Python 3.9
17:17:20 stephenfin gmann: However, in the past the functional tests didn't work with Python 3.9. It could be possible that things have been fixed since
17:18:05 stephenfin gmann: However, regardless, there's no reason we should be running different things locally and in the CI. Something could conceivably pass locally (where we're using Python 3.9) but fail in the gate (using Python 3.8)
17:18:36 stephenfin gibi: done
17:18:40 gmann stephenfin: yeah, l-c can be dropped :) which i am not much worried about.
17:19:36 gmann stephenfin: cases like passing py3.9 and failing py3.8 should not be much right as at next cycle we want all code to run on both
17:20:12 gmann stephenfin: i am not against of that change to test py3.8 as default locally but it just add extra work you mentioned in commit msg

Earlier   Later