Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2022-06-22
13:24:26 opendevreview Amit Uniyal proposed openstack/nova master: Adds validation for hw machine type in host caps https://review.opendev.org/c/openstack/nova/+/847126
13:26:45 gibi Uggla: responded, fine with bauzas' suggestions
13:31:26 bauzas gibi: Uggla: replied too
13:33:25 bauzas Uggla: we already this kind of sentinel for example in https://github.com/openstack/nova/blob/master/nova/scheduler/manager.py#L114
13:34:15 bauzas that's how we flag an unset param
13:34:59 bauzas this is a simple pattern
13:38:50 gibi I think the special case here is that the caller needs differentiate too if some AZ value is needed to pass forward or the param should not be provided
13:39:58 bauzas gibi: not sure I understand
13:40:09 bauzas gibi: either the AZ is passed or not
13:40:15 bauzas but the value can be None
13:40:16 bauzas right?
13:40:38 gibi by making the new_az optional the caller either needs to pass a value (including None) or not pass the parameter at all
13:40:58 gibi that will be either a conditional on the caller
13:41:17 bauzas https://specs.openstack.org/openstack/nova-specs/specs/zed/approved/unshelve-to-host.html#proposed-change
13:41:19 gibi or the caller could pass the sentinel and avoif the conditional
13:41:35 bauzas ah I see your concern
13:41:47 bauzas well, in general we do this kind of conditional
13:41:48 gibi I'm fine with that extra conditional on the caller side
13:42:00 bauzas with kwargs
13:42:21 bauzas like kwargs = {}
13:42:29 gibi also I'm fine passing the sentinel and avoid the conditional too
13:42:35 bauzas api.unshelve(ctxt, inst, host, **kwargs)
13:42:52 gibi ahh, OK, you can do that too
13:42:56 bauzas and if az is set, then kwargs['new_az'] = this
13:43:09 bauzas that's the general pattern we have for sentinels
13:43:44 bauzas no need to have different calls then
13:43:52 gibi I'm OK with the kwargs way too
13:43:53 bauzas one single call but with optional args
13:44:17 bauzas gibi: well, as I said, this is the pattern we have in Nova for a while now :)
13:44:29 bauzas ie. default a param to a sentinel value
13:44:35 bauzas and call the method with kwargs
13:44:52 bauzas Uggla: ^
13:45:43 gibi I can argue about how well established this as a pattern based on I'm not being aware of it. But I guess it is irrelevant. :)
13:45:59 gibi as I agree with the actual code
13:47:12 bauzas https://github.com/openstack/nova/blob/master/nova/scheduler/rpcapi.py#L152
13:47:19 bauzas and https://github.com/openstack/nova/blob/master/nova/scheduler/manager.py#L145
13:47:40 bauzas gibi: the problem with importing a global var from another module is about any possible circular import
13:47:52 bauzas if we don't need to import, let's not do it
13:48:14 gibi sure
13:48:17 bauzas also, the caller has to have some knowledge about the method
13:48:29 gibi as I said I agree with the code :)
13:48:34 bauzas this is a bad behaviour
13:48:41 bauzas gibi: sure, I just explain why
13:48:56 bauzas mostly not for you
13:49:00 bauzas but also for Uggla :)
13:49:45 bauzas so, yeah, unnecessary module import and internal knowledge of the called method are the two things we want to avoid
13:50:11 bauzas and since optional params in python methods are something easy to do, let's use this pattern
13:53:14 Uggla hum I think I understand.
13:53:27 gibi I think we use None a lot more to signal information-not-provided than a sentinel value. We only use dedicated sentinel if None means something else than information-not-provided
13:53:47 gibi and in this case None means something else than information-not-provide4d
13:54:25 bauzas gibi: correct
13:54:44 bauzas in general, we set to None unless None is used for a specific flag
13:54:52 gibi yepp
13:54:56 bauzas and if so, we use the sentinel pattern
13:55:49 bauzas but again, we create a specific instance of an object that we use, and we try to not expose this instance elsewhere
13:56:13 bauzas by instance, I mean a stored value in memory
14:05:30 gibi purely hypotetically and purely from the code understandability perspective I'm not in favor of the optionalness of a parameter on an API. In python you have to look up the signature of the called function to know if a parameter is optional (i.e. has a default value in the signature) but as soon as you looked that up you see the default value (the sentinel) so that default value already leaked to the
14:05:36 gibi caller side
14:06:35 gibi I would make all the parameters non defaulted and document what value of what parameter means what :D
14:06:54 gibi that is a bit more explict than param=sentinel in a signature
14:07:16 gibi but this is way less important that make the unshelve patch land :D
14:07:24 gibi s/that/than/
14:07:42 bauzas that's why I explained why the pattern and also why I said in my last comment that docstrings help
14:08:12 bauzas I'd rather see a docstring saying (optional) my param
14:08:43 bauzas rather than asking to import a specific instance of a global class object
14:10:46 gibi the function itself is a global on the class an you import that to be able to call it
14:11:19 gibi None is an interpreter global :)
14:11:33 gibi (we are getting philosophycal )
14:12:33 gibi we are using enums from the fields module those are class level fields too
14:26:49 Uggla gibi, bauzas, last stuff, ok with FIELD_SENTINEL wording ?
14:29:48 gibi go with what bauzas asked for I won't block on it
14:30:29 bauzas Uggla: gibi: you can even name it privatly
14:30:36 bauzas like _unset_field
14:30:45 gibi sure
14:30:48 bauzas or _unsel_field_sentinel
14:31:01 bauzas I don't wanna nitpick on the naming :)
14:31:22 Uggla naming one of the hardest stuff in computing.
14:31:30 bauzas but yeah, actually, since this is an internal object, make it private
14:31:43 bauzas (by convention of course)
14:32:27 bauzas https://twitter.com/codinghorror/status/506010907021828096?lang=fr
14:32:31 bauzas :)
14:32:44 bauzas (I like this tweet :p )
14:33:38 bauzas https://www.karlton.org/2017/12/naming-things-hard/ for the wider context
17:11:44 opendevreview Amit Uniyal proposed openstack/nova master: Adds validation for hw machine type in host caps https://review.opendev.org/c/openstack/nova/+/847126
19:45:42 opendevreview Amit Uniyal proposed openstack/nova master: Adds validation for hw machine type in host caps https://review.opendev.org/c/openstack/nova/+/847126
20:38:09 ade_lee sean-k-mooney, sean-k-mooney[m] slaweq stephenfin hey - any idea what might be causing failures here? https://8767bac9cdd8c58da256-ee4c5d809145a8a3246cc4d26d65fbe0.ssl.cf5.rackcdn.com/831844/10/experimental/tempest-centos9-stream-fips/45be779/testr_results.html
20:41:16 ade_lee oh -- maybe thats this -- https://bugs.launchpad.net/neutron/+bug/1979047
#openstack-nova - 2022-06-23
05:28:12 frickler ade_lee: yes, that's the same failure pattern that caused us to make c9s non-voting in devstack
08:27:14 bauzas gibi: others, fwiw, I'll be on PTO tomorrow
08:27:24 gibi bauzas: o/ ack
09:15:51 opendevreview Amit Uniyal proposed openstack/nova master: Adds validation for hw machine type in host caps https://review.opendev.org/c/openstack/nova/+/847126
10:06:59 sean-k-mooney ade_lee: assuming this is related to fips and centos 9 stream ya interface attach is broken due to an upstream libvirt bug which has a fix internally but is not relased yet
11:06:03 opendevreview Merged openstack/nova master: Change TooOldComputeService upgrade check code to failure https://review.opendev.org/c/openstack/nova/+/845262
14:08:12 opendevreview Amit Uniyal proposed openstack/nova master: Adds validation for hw machine type in host caps https://review.opendev.org/c/openstack/nova/+/847126
14:31:20 amorin hello nova!
14:34:55 amorin I was looking for an idea on how to schedule instances on computes based on some metrics that are not cpu related
14:35:04 amorin like, the kernel version, the operating system version
14:40:35 amorin i was thinking that I could tweak the compute monitors (that collect metrics)
14:40:40 amorin but I dont know if this is the best idea?
14:45:05 mnaser amorin: maybe you could use traits out of bands?

Earlier   Later