| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-05-15 | |||
| 09:31:30 | stephenfin | cool | |
| 09:31:36 | brinzhang | stephenfin: do I need to remove this sentence from the commit message? https://review.opendev.org/#/c/727589/8//COMMIT_MSG | |
| 09:31:44 | stephenfin | also, is it just me, or is Gerrit really slow this week? | |
| 09:31:55 | lyarwood | stephenfin: just you | |
| 09:31:55 | stephenfin | like, 'git review -d foo' takes minutes | |
| 09:31:59 | stephenfin | damn | |
| 09:32:23 | stephenfin | the web UI is a-ok, but pulling stuff is taking an age | |
| 09:32:24 | brinzhang | git review -d, I always need minutes | |
| 09:32:39 | lyarwood | I've really not noticed anything tbh | |
| 09:33:07 | stephenfin | brinzhang: you don't need to exclude F811 now, do you? | |
| 09:33:13 | stephenfin | gibi: :O | |
| 09:33:28 | stephenfin | so you manually copy download links? | |
| 09:33:34 | stephenfin | *checkout links | |
| 09:33:57 | gibi | stephenfin: yepp, and I use git push origin HEAD:refs/for/master/my-topic to push things up | |
| 09:34:07 | brinzhang | stephenfin: you mean I should remove F811 from flake8 ignore in tox.ini? | |
| 09:34:36 | stephenfin | brinzhang: I assume so, since you added noqa lines to the "duplicated" functions/methods? | |
| 09:34:47 | stephenfin | That is what F811 is for, right? | |
| 09:34:57 | brinzhang | yes | |
| 09:35:39 | brinzhang | in the future, if someone need to add one function name as the same as before, they also need to add noqa. | |
| 09:35:46 | brinzhang | ok, I will remove F811 from tox | |
| 09:36:22 | brinzhang | gibi: IMO, you can give a guide to use link to submit code, and update code. | |
| 09:36:38 | gibi | stephenfin: I have full control this way | |
| 09:36:51 | brinzhang | I always used 'git review ', it seems slowly always | |
| 09:37:00 | gibi | stephenfin: e.g. I can push patches from a series during interactive rebase | |
| 09:37:22 | stephenfin | gibi: you mean half way through the rebase? | |
| 09:37:46 | gibi | stephenfin: yepp | |
| 09:38:00 | gibi | stephenfin: for long chains it helps | |
| 09:38:26 | stephenfin | Fair. I'd usually just download patch N locally and rebase all of that | |
| 09:38:34 | gibi | stephenfin: yeah that works too | |
| 09:39:07 | openstackgerrit | Brin Zhang proposed openstack/nova master: Bump hacking min version to 3.1.0 https://review.opendev.org/727589 | |
| 09:39:13 | gibi | I don't see what really git review automates that I cannot do manually with git with significant extra burden | |
| 09:39:38 | gibi | I mean plain git seems as easy as git review to me | |
| 09:41:07 | stephenfin | Can't argue with that | |
| 09:48:38 | gibi | :) | |
| 10:05:08 | openstackgerrit | Harshavardhan Metla proposed openstack/nova master: [Nova] Add reference to Placement installation guide https://review.opendev.org/726936 | |
| 10:06:42 | bauwser | fwiw, git-review does more than just uploading your series | |
| 10:08:16 | bauwser | at least automatically rebasing on top of the remote | |
| 10:22:47 | openstackgerrit | Jiri Suchomel proposed openstack/nova master: Adapt the admin guide to describe the direct RBD export https://review.opendev.org/728411 | |
| 11:57:31 | sean-k-mooney | stephenfin: add mypy to the linters target i was suggesting lyarwood add bashate too and make it non voteing for now? | |
| 11:58:25 | sean-k-mooney | stephenfin: then we can either move it to pep8 when we want to make it required or we can make the linters target voting to make both voting | |
| 12:01:52 | lyarwood | sean-k-mooney: I didn't even think about actually using a linters target | |
| 12:03:51 | sean-k-mooney | lyarwood: yep you should. i left a comment to that effect on your patch but it took me like an hour to realise that is where you should put the bashate command too | |
| 12:04:35 | sean-k-mooney | given the linter tox enve and job are a semi standard now, they are just not required in the pti | |
| 12:04:41 | sean-k-mooney | at least not yet anyway | |
| 12:07:48 | lyarwood | sean-k-mooney: yeah I've just seen your reviews on that bashate change, missed them earlier. | |
| 12:08:19 | lyarwood | sean-k-mooney: I've already ack'd stephenfin's initial change but we can follow up and move it into a linters target later | |
| 12:09:10 | sean-k-mooney | where is his mypy change landing currently? | |
| 12:09:43 | lyarwood | sean-k-mooney: https://review.opendev.org/#/c/676208/ | |
| 12:10:00 | sean-k-mooney | to be clear i was pretty ok with it landing in pep8 straight away but i dont think it deserves its own job or tox target | |
| 12:10:28 | sean-k-mooney | ok i see | |
| 12:10:30 | lyarwood | sean-k-mooney: kk | |
| 12:10:38 | sean-k-mooney | its just a seperate target but not added to any job | |
| 12:10:40 | sean-k-mooney | that works too | |
| 12:11:01 | sean-k-mooney | but ya linter i think is the way to go | |
| 12:11:36 | sean-k-mooney | stephenfin: by the way if you want to add mypy to os vif i wont say no :) | |
| 12:16:10 | jsuchome | hey ... I do not know what are you discussing, is it related to the fact that pep8 builds are failing with errors unrelated to the patch? | |
| 12:16:28 | sean-k-mooney | jsuchome: no | |
| 12:16:46 | sean-k-mooney | jsuchome: mypy is a checker for python type annotations | |
| 12:17:32 | sean-k-mooney | jsuchome: we are slowly starting to adopt it now that we are python3 only and can do so more easily | |
| 12:17:48 | lyarwood | jsuchome: https://review.opendev.org/727347 should fix the pep8 errors | |
| 12:18:20 | jsuchome | I see, thanks | |
| 12:39:16 | openstackgerrit | Andreas Jaeger proposed openstack/python-novaclient master: Switch to newer openstackdocstheme and reno versions https://review.opendev.org/728270 | |
| 12:39:19 | openstackgerrit | Andreas Jaeger proposed openstack/nova master: Switch to newer openstackdocstheme and reno versions https://review.opendev.org/728263 | |
| 12:58:57 | openstackgerrit | Stephen Finucane proposed openstack/nova master: hardware: Update and correct typing information https://review.opendev.org/714694 | |
| 12:59:33 | stephenfin | lyarwood: Addressed your comments on ^ | |
| 13:07:45 | lyarwood | stephenfin: ack thanks | |
| 13:08:54 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Reserve DISK_GB resource for the image cache https://review.opendev.org/728439 | |
| 13:09:39 | gibi | dansmith: hi! fyi there is the first version of disk reservation patch for the image cache ^^ | |
| 13:10:45 | gmann | gibi: thanks | |
| 13:11:51 | francoisp | lyarwood, bauzas hello, when you get a chance: https://review.opendev.org/#/c/726508/ -thank you! | |
| 13:13:00 | lyarwood | francoisp: ack sorry done | |
| 13:15:50 | francoisp | thanks lyarwood | |
| 13:25:20 | openstackgerrit | Merged openstack/nova master: Bump hacking min version to 3.0.1 https://review.opendev.org/727347 | |
| 13:33:52 | openstackgerrit | Jiri Suchomel proposed openstack/nova master: Bring back allowed_direct_url_schemes in support of RBD image download https://review.opendev.org/728095 | |
| 13:35:48 | gmann | finally merged. gate should be green now | |
| 13:41:35 | openstackgerrit | Jiri Suchomel proposed openstack/nova master: Adapt the admin guide to describe the direct RBD export https://review.opendev.org/728411 | |
| 13:44:17 | dansmith | gibi: you decided on reserved instead of a new allocation? | |
| 13:44:43 | gibi | dansmith: yes, reservation is the current working assumption | |
| 13:44:44 | dansmith | gibi: I really wasn't arguing that reserved was better (and I think I said that) I just wanted us to consider it | |
| 13:45:51 | dansmith | gibi: it certainly looks simpler than a new allocation though. I was going to look for code that deleted the allocation if they moved the cache from shared to separate filesystem, but this would auto correct itself | |
| 13:46:11 | gibi | dansmith: I realized that we recalculate reservation at every update_provider_tree() call anyhow | |
| 13:46:20 | dansmith | yeah | |
| 13:46:39 | gibi | so I plugged my calculation into that loop | |
| 13:47:23 | dansmith | yeah, even though this is less granular, it feels like a much smaller and less scary change, even something we could backport | |
| 13:47:52 | gibi | there is one downside, placement allow bumping the reservation over the total - usage | |
| 13:48:44 | dansmith | the other downside is this lags the actual usage | |
| 13:48:50 | dansmith | but, very simple | |
| 13:49:59 | gibi | when I pushed the patch I added a TODO to bump the reservation before we download an image but since then I lost in the imagebackend code again to find the place where we download image to the cache and it is faaar from easy | |
| 13:50:21 | dansmith | yeah, no | |
| 13:52:00 | gibi | I start thinking that from the reservation handling perspective this is what we can do. This is far from perfect but it has a good roi | |
| 13:53:19 | gibi | dansmith: would it be OK with you if we stop in the virt driver at this point? I will still look into adding the pre-filter part separately | |
| 13:53:43 | gibi | but I'm scared from the imagebackend | |
| 13:54:37 | dansmith | what do you mean "stop in the virt driver"? | |
| 13:55:49 | gibi | I mean, not trying to bump the reservation before we download a new image to the cache | |
| 13:57:09 | dansmith | oh yeah, I don't think that's the way to go.. we'd have to update inventory before each boot | |
| 13:57:50 | dansmith | I think if we go the reservation approach, we accept the imperfectness of it, in exchange for its simplicity | |
| 13:58:10 | dansmith | if it turns out to not help often enough, then we can move to the allocation method or something else | |
| 13:58:17 | dansmith | a pre-filter will help avoid that | |
| 13:58:41 | dansmith | I dumped some comments about the approaches on the patch just now | |
| 13:58:46 | gibi | looking | |