| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-06-22 | |||
| 10:00:37 | bauzas | sean-k-mooney: we had this workflow since 2013 IIRC | |
| 10:00:46 | bauzas | and it was working | |
| 10:00:57 | bauzas | we only need one single nova-core review | |
| 10:01:33 | sean-k-mooney[m] | sure it works i just dont think its the right way to do things | |
| 10:01:44 | bauzas | https://translate.openstack.org/project/view/nova?dswid=-7176 | |
| 10:02:08 | bauzas | sean-k-mooney: if you have concerns, then you should be discussing with the i18n team, not here | |
| 10:02:24 | sean-k-mooney[m] | why | |
| 10:02:24 | Uggla | gibi, thx, I think it is ok. | |
| 10:02:40 | sean-k-mooney[m] | i can but this is also a project desicion | |
| 10:02:44 | sean-k-mooney[m] | and discussion | |
| 10:03:27 | sean-k-mooney[m] | im wondering if we still want to ship tanslations as a project in tree and if we should change that going forward and remove them form the souce tree | |
| 10:04:23 | bauzas | sean-k-mooney: because it's a TC question https://docs.openstack.org/project-team-guide/i18n.html | |
| 10:04:38 | bauzas | and again, this is simple and it works | |
| 10:04:55 | bauzas | anyway, I merged it | |
| 10:05:10 | bauzas | I could even write some translations from French, if that helps :) | |
| 10:05:44 | bauzas | Only 43% of Nova is translated in French :p | |
| 10:07:10 | sean-k-mooney[m] | if you want too :P | |
| 10:16:11 | gibi | we need more French! | |
| 10:19:20 | opendevreview | Merged openstack/nova master: Imported Translations from Zanata https://review.opendev.org/c/openstack/nova/+/846876 | |
| 11:00:55 | mnasiadka | Hello - is there a way to specify Cinder volume-type for boot from volume in flavour extra-specs? | |
| 11:35:09 | gibi | mnasiadka: you have to pre-create the volume in cinder with the desired volume-type and then pass the volume to nova to boot from | |
| 12:04:31 | mnasiadka | gibi: oh ok, thanks | |
| 12:21:45 | bauzas | gibi: Uggla: sean-k-mooney : can we punt our session by 30 mins ? I have something at the moment | |
| 12:22:04 | sean-k-mooney | i wont be able to do it sorry | |
| 12:22:10 | sean-k-mooney | im in a meeting till the top of the hour | |
| 12:22:10 | bauzas | ie. 15:00 for CEST and 14:00 for BSR | |
| 12:22:15 | bauzas | BSTR* | |
| 12:22:19 | bauzas | graaah | |
| 12:22:21 | bauzas | BST* | |
| 12:22:24 | sean-k-mooney | and then i have a dental apointmnt in an hour | |
| 12:22:43 | bauzas | ok, then let's discuss about Uggla's spec later then | |
| 12:22:43 | sean-k-mooney | so it would have to be later today or you and gibi can proceed with Uggla without me | |
| 12:22:51 | gibi | later then | |
| 12:23:07 | gibi | but as far as I see sean-k-mooney stated his oppinion above and I agreed | |
| 12:24:16 | Uggla | bauzas, sean-k-mooney bauzas, I better understand sean-k-mooney proposal that sounds good. | |
| 12:25:31 | bauzas | kk | |
| 12:25:37 | sean-k-mooney | i just pushed my comments on the sepc too | |
| 13:09:46 | Uggla | bauzas, gibi if you can have a look at my latest comments on https://review.opendev.org/c/openstack/nova/+/831507. On 2 points your opinions diverge and I can make both of you pleased at the same time. So I need a consensus between both of you to implement the solution you agree on. | |
| 13:13:50 | bauzas | Uggla: sure, will look | |
| 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 | |