| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-02-05 | |||
| 16:52:34 | efried | oh, didn't mriedem propose a WIP that started doing that? | |
| 16:52:46 | dansmith | he complained about it a lot, so probably | |
| 16:53:26 | efried | https://review.opendev.org/#/c/697686/ ? | |
| 16:53:38 | efried | no, not quite | |
| 16:53:49 | dansmith | well, that's what you're thinking of probably | |
| 16:54:11 | efried | yeah | |
| 16:54:15 | dansmith | regardless, as you can see, from_components is just "set a bunch of things and no other logic" | |
| 16:54:49 | efried | yeah. but it's common to both build and cold migrate | |
| 16:55:24 | dansmith | to what end? | |
| 16:55:33 | efried | just DRY | |
| 16:55:39 | dansmith | it's not really though | |
| 16:55:46 | efried | like, I'm not sure I see a better split logically. Whether we want to get rid of the filter_properties in some way, that's fine | |
| 16:55:49 | dansmith | because the actual constructor can take field values | |
| 16:56:20 | efried | There's a *little* bit of logic in there. | |
| 16:56:24 | efried | All these _from_* methods | |
| 16:57:03 | dansmith | okay, I guess so | |
| 16:57:20 | dansmith | but that would also kinda mean that the setting of a field that doesn't need any handling shouldn't be in here | |
| 16:58:55 | efried | reqspec.fold_in_other_shit(filter_properties, maybe_others, ...) | |
| 16:58:55 | efried | for all foo/bar that are static, and then | |
| 16:58:55 | efried | reqspec = RequestSpec(foo=bar, ...) | |
| 16:58:55 | efried | So your refactor would be like the caller doing | |
| 16:59:42 | efried | where the latter is from_components with all the static setters removed? | |
| 17:00:31 | dansmith | well, that's a potential bigger refactor, because from_components is a classmethod | |
| 17:00:50 | dansmith | and, I'm not sure that would really work with the way the _from things work.. they may expect to "go first" | |
| 17:01:24 | dansmith | so I would just take the req_spec that comes from from_components, and follow up with the direct things | |
| 17:01:36 | dansmith | req_spec = from_components(..); req_spec.requested_resources = port_requested_resources + accel_requested_resources; etc | |
| 17:06:08 | efried | I guess I don't see the benefit of assigning some fields in the classmethod and some afterwards. Similar reasoning as mentioned previously: then you always have to go make sure you're assigning the right pieces the right way on the right side of the method boundary. | |
| 17:06:29 | efried | but I don't feel strongly enough to -1 that. | |
| 17:06:44 | mriedem | beware the request spec quagmire | |
| 17:06:54 | efried | I think we're already quagged. | |
| 17:07:58 | mriedem | if you go one up that stack https://review.opendev.org/#/c/697697/ and read my comments, unwinding a bunch of that stuff is going to be dependent on doing a major conductor rpc api version | |
| 17:08:12 | mriedem | because there is backward compat code to handle the older version | |
| 17:08:20 | mriedem | which relies on a lot of this bridge code | |
| 17:08:36 | dansmith | efried: I commented and I'm late for another meeting | |
| 17:11:00 | stephenfin | melwitt: Does my reply at https://review.opendev.org/#/c/705654/ make sense? | |
| 17:11:03 | stephenfin | before I rewrite the thing | |
| 17:12:18 | larsks | asd | |
| 17:15:44 | melwitt | stephenfin: oh, wow. so ... we are ok to remove the helper method so long as the minimum python version is 3.6 because the native method is doing the same thing? are we (deployers) guaranteed to not be running <= 3.6 at this point? | |
| 17:16:15 | stephenfin | melwitt: yup https://github.com/openstack/nova/blob/master/setup.cfg#L9 | |
| 17:16:25 | stephenfin | you can't install nova on anything less | |
| 17:16:30 | melwitt | stephenfin: cool, thanks | |
| 17:16:53 | melwitt | yup, that all makes sense then | |
| 17:17:20 | stephenfin | appears to have been the behavior since Python 3.3 | |
| 17:17:29 | stephenfin | https://github.com/python/cpython/blob/v3.3.0/Lib/http/server.py#L562-L565 | |
| 17:17:39 | stephenfin | cool, I'll rework that so. Thanks for the review | |
| 17:28:19 | melwitt | stephenfin: I just looked this up, apparently the original commit you referenced _would_ fix the issue because the reverse dns lookup was happening as a result of simply logging a message (I did not expect this) https://github.com/openstack/nova/commit/c0f773a616fb48bf699539c5ac18bd9c55a540c9 | |
| 17:30:03 | stephenfin | melwitt:Oh, fun. Fancy suggesting a commit message I should use? :) | |
| 17:32:22 | melwitt | stephenfin: technically your commit message is right, so I think we can leave it. let me just add a comment and change my vote | |
| 17:35:30 | stephenfin | melwitt: Okay. I do need to repush that series to address gibi's comments on the base patch though so if I do need to edit the commit message, let me know in the next 2 mins :) | |
| 17:39:08 | melwitt | stephenfin: k, I think it's fine as-is | |
| 17:43:12 | openstackgerrit | Stephen Finucane proposed openstack/nova master: trivial: Remove unused 'cache_utils' APIs https://review.opendev.org/705652 | |
| 17:43:13 | openstackgerrit | Stephen Finucane proposed openstack/nova master: trivial: Bump minimum version of websockify https://review.opendev.org/705654 | |
| 17:43:13 | openstackgerrit | Stephen Finucane proposed openstack/nova master: trivial: Fetch 'Service' objects once when building AZs https://review.opendev.org/705653 | |
| 17:43:14 | openstackgerrit | Stephen Finucane proposed openstack/nova master: trivial: Remove 'run_once' helper https://review.opendev.org/705656 | |
| 17:43:14 | openstackgerrit | Stephen Finucane proposed openstack/nova master: trivial: Merge unnecessary 'NovaProxyRequestHandlerBase' separation https://review.opendev.org/705655 | |
| 17:43:15 | openstackgerrit | Stephen Finucane proposed openstack/nova master: mypy: Add type annotations to 'nova.pci' https://review.opendev.org/676209 | |
| 17:43:15 | openstackgerrit | Stephen Finucane proposed openstack/nova master: tox: Integrate mypy https://review.opendev.org/676208 | |
| 17:43:16 | openstackgerrit | Stephen Finucane proposed openstack/nova master: WIP: mypy: Add type annotations to top-level modules https://review.opendev.org/705658 | |
| 17:43:16 | openstackgerrit | Stephen Finucane proposed openstack/nova master: mypy: Add nova.cmd, nova.conf, nova.console https://review.opendev.org/705657 | |
| 17:47:54 | stephenfin | I, however, am outta here | |
| 17:47:54 | stephenfin | melwitt: There's a super trivial follow-up to that too, btw, if you'd like to take a look https://review.opendev.org/#/c/705655/ | |
| 17:48:37 | efried | sean-k-mooney (sean-k-mooney1): https://blueprints.launchpad.net/nova/+spec/config-tsc-freq looks like something you would be able to grok. I talked to umbSublime and he's interested in working the implementation. At a glance, it seems like it should be fairly straightforward, but I'd like your opinion if you've got a moment to look. | |
| 17:48:46 | melwitt | stephenfin: will do | |
| 17:49:16 | sean-k-mooney | ya i was ment to write up a spec/blueprint | |
| 17:49:21 | sean-k-mooney | or help them | |
| 17:50:52 | sean-k-mooney | efried: the propoasal was to basically to add 2 extra specs one for the tsc frequency and the other for enableing invtsc | |
| 17:51:56 | sean-k-mooney | also add a trait to land on host that support invtsc | |
| 17:52:17 | efried | sean-k-mooney: cool. This would be umbSublime's first contribution, so he'll need some mentoring... | |
| 17:53:37 | sean-k-mooney | looking at my browser history we created 1 an i think i was ment to help draft the update to the bluepirnt. | |
| 17:53:41 | sean-k-mooney | https://etherpad.openstack.org/p/invtsc | |
| 17:53:56 | sean-k-mooney | is umbSublime about | |
| 17:54:18 | efried | That etherpad looks like the same content as what's in the bp description. | |
| 17:54:21 | sean-k-mooney | my isp decied to force recet my router again and broke my internet | |
| 17:54:23 | sean-k-mooney | yes | |
| 17:54:37 | sean-k-mooney | so i obvioulsy never wrote the updated draft | |
| 17:54:40 | efried | from what I could glean, "we" decided at some point that the blueprint could be specless. | |
| 17:54:54 | sean-k-mooney | yes i brought it up in the nova meeting | |
| 17:55:15 | sean-k-mooney | and we said if we spell out the minor changes need then it could be specless | |
| 17:55:22 | efried | great | |
| 17:56:52 | sean-k-mooney | umbSublime: this sliped off my radar sorry. | |
| 17:57:41 | umbSublime | reporting in o/ | |
| 17:58:04 | sean-k-mooney | umbSublime: if you are still up for working on the code cahgne ill try and draft some worth in the ether path to describe what is needed. if you are happy with tem we can update the blueprint and i can help you with reviewing your patch | |
| 17:58:57 | umbSublime | Sounds good to me! I don't think I'll have much time to work on this this week, but next week should be fine! | |
| 17:59:34 | umbSublime | sean-k-mooney, in which etherpad would that be ? | |
| 17:59:56 | efried | sean-k-mooney, umbSublime: we'll want something in approvable form by next week (spec freeze). | |
| 18:00:08 | sean-k-mooney | https://etherpad.openstack.org/p/invtsc | |
| 18:00:15 | umbSublime | efried, I see | |
| 18:00:30 | sean-k-mooney | umbSublime: so looking at my private irc logs from talking to you in novemeber | |
| 18:01:05 | sean-k-mooney | i think the proposal was to add a hw:tsc_freq_mhz flavor extra spec | |
| 18:01:40 | sean-k-mooney | and a hw:invtsc=on|off extra spec | |
| 18:02:12 | sean-k-mooney | that would be how you requested the feature i the flavor | |
| 18:02:41 | umbSublime | yes iirc that's we had discussed last time. I'll have to read back my notes to | |
| 18:02:58 | sean-k-mooney | form a shcduler point of view we would need a new trait for invtsc support | |
| 18:03:17 | sean-k-mooney | which would be auto added if you enabled the flavor extra spec | |
| 18:03:41 | sean-k-mooney | and then the last bit was just the libvirt driver chagne to add the xml elements and report support | |
| 18:04:43 | mriosfer | Hi , where i can find a suported list of gpu supported by queens? | |
| 18:05:01 | mriosfer | sean: looks like hw:video_ram dont report vram to windows for some reason :S | |
| 18:05:07 | sean-k-mooney | umbSublime: do you want to take go at writing that up? or shall i? | |
| 18:05:42 | sean-k-mooney | umbSublime: i also linked https://review.opendev.org/#/c/671338/ to you as an example of how to add the feature just to refresh your memory | |
| 18:06:22 | sean-k-mooney | mriosfer: so in the image its hw_video_ram and you need to set hw_video:max_ram in the flavor i think | |