| *** mhen_ is now known as mhen | 02:00 | |
| *** trident- is now known as trident | 09:49 | |
| sean-k-mooney | atmark: we have never supproted detacting volume via cinder like that and it actully cause security problem if you do that | 10:38 |
|---|---|---|
| sean-k-mooney | atmark: the only way to detach a volume form an instnace is via nova's api | 10:39 |
| sean-k-mooney | depending on the relase of openstack we have two volume healing operations, in newer relase a hard reboot will regenerate teh bdms using cidner as the source of truth for what shoudl be attached, we also have a nova-manage command that predates it and uses the bdm as the souce of truth and will recreate the attachments | 10:41 |
| opendevreview | Takashi Kajinami proposed openstack/nova stable/2024.1: hardware: Fix image_meta.id within get_mem_encryption_constraint https://review.opendev.org/c/openstack/nova/+/960050 | 13:26 |
| opendevreview | Takashi Kajinami proposed openstack/nova stable/2024.1: libvirt: Disable VMCoreInfo device for SEV-encrypted instances https://review.opendev.org/c/openstack/nova/+/959914 | 13:27 |
| opendevreview | Merged openstack/python-novaclient master: Update master for stable/2025.2 https://review.opendev.org/c/openstack/python-novaclient/+/959749 | 13:38 |
| opendevreview | Takashi Kajinami proposed openstack/nova stable/2025.1: libvirt: Disable VMCoreInfo device for SEV-encrypted instances https://review.opendev.org/c/openstack/nova/+/958874 | 13:40 |
| opendevreview | Takashi Kajinami proposed openstack/nova stable/2024.2: libvirt: Disable VMCoreInfo device for SEV-encrypted instances https://review.opendev.org/c/openstack/nova/+/959913 | 13:41 |
| dansmith | I'm sick of autopep8 reformatting my code instead of suggesting it when I run `tox -epep8` | 14:45 |
| dansmith | surely there's some way I can locally prefer not to do that right? | 14:45 |
| dansmith | bauzas: Uggla ? | 14:45 |
| dansmith | not sure when the team agreed to do this, but man it's annoying | 14:45 |
| bauzas | dansmith: pre-commit is already doing that :( | 14:47 |
| dansmith | I know | 14:47 |
| dansmith | I want to turn it off | 14:47 |
| bauzas | apart finding another agreement, I don't see what we can do | 14:47 |
| bauzas | the agreement being that autopep8 is and should always be optional | 14:48 |
| bauzas | including in the tox target | 14:48 |
| dansmith | bauzas: +1000 | 14:48 |
| bauzas | also, skipping pre-commit is unfair, as I need pre-commit for adding the gerrit change ID automatically, I can't just skip autopep8 | 14:48 |
| bauzas | (AFAIK, no way to only skip a subset of the multiple pre-commit hooks) | 14:49 |
| Uggla | hum it is the case for quite some time right ? | 14:49 |
| dansmith | Uggla: it has been yes and I've hated it this whole time, but this morning it's really killing me and I want it to stop | 14:50 |
| dansmith | I also argued against it when it was even proposed before it was autopep8'ing | 14:50 |
| dansmith | IIRC it used to just show the suggestions but not force them on me | 14:51 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Rally job for eventlet-removal https://review.opendev.org/c/openstack/nova/+/960130 | 14:59 |
| Uggla | it depends some changes are left to the author, but some basic reformatting are applied, if I'm not wrong. | 14:59 |
| dansmith | yes and I never ever want it to do that.. | 14:59 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Rally job for eventlet-removal https://review.opendev.org/c/openstack/nova/+/960130 | 15:00 |
| dansmith | if I have uncommitted changes and I run pep8, it will make a bunch of extra changes that I did not want that are all "opinions" of the tool and now I have more change queued | 15:00 |
| dansmith | right now it keeps forcing this on me: 3 | 15:01 |
| dansmith | https://paste.opendev.org/show/bjPrI7fQi2rx3TH1ndDB/ | 15:01 |
| dansmith | which takes the code from readable to stupid for no effing reason | 15:02 |
| ratailor__ | bauzas, melwitt can I get reviews on backports https://review.opendev.org/c/openstack/nova/+/958834 and https://review.opendev.org/q/project:openstack/nova+branch:stable/2024.2+owner:ratailor@redhat.com | 16:10 |
| opendevreview | Takashi Kajinami proposed openstack/nova stable/2024.1: libvirt: Disable VMCoreInfo device for SEV-encrypted instances https://review.opendev.org/c/openstack/nova/+/959914 | 16:12 |
| sean-k-mooney | bauzas: the change-id hook is entirly seperate | 16:20 |
| sean-k-mooney | you can 1 not install pre-commit, 2 if you have it install do --no-verify to dsiabll all hooks other then the change id hook ro 3 use SKIP=list,of,hook.ids as some options | 16:21 |
| bauzas | sean-k-mooney: sorry, I meant that --no-verify skips all the hooks | 16:21 |
| sean-k-mooney | all the hooks form the pre-commit tool but i dont think it skips the change id hook | 16:21 |
| sean-k-mooney | dansmith: that may be a side effect of a change i did a while a go to make autopep8 fix more things | 16:22 |
| sean-k-mooney | we coudl look at trunign that off | 16:22 |
| dansmith | please please please turn it off | 16:23 |
| bauzas | sean-k-mooney: the git help says that it also skims commit hooks, so the gerrit one | 16:23 |
| sean-k-mooney | ack | 16:23 |
| bauzas | commit-msg sorry | 16:23 |
| sean-k-mooney | let me find the comit and pose a revert and see if that help | 16:23 |
| sean-k-mooney | i basically took some of the config form oslo/sdk and ported it a few monts ago | 16:24 |
| dansmith | fast8 also was broken, I assume by one of these changes a while ago which is also really frustrating | 16:25 |
| dansmith | I haven't tried fixing it yet, but it's really annoying because it's like "please try pre-commit instead!" with the implication of "so I can break your code for you" | 16:25 |
| sean-k-mooney | well that was ment to bedeelted a while ago but i dont think that is related | 16:25 |
| dansmith | I dunno why it was "meant to be deleted" .. I assume that's just your opinion, but it's a huge time saver | 16:26 |
| sean-k-mooney | so this https://github.com/openstack/nova/commit/ebd75760e1907bb4b1bc089ecf6f4405bf52e208 might be the issue | 16:27 |
| sean-k-mooney | dansmith: not just that it has not been maintaiend for a while an i otught it broke with some of the ohter seuptools change ectra | 16:27 |
| sean-k-mooney | i dont reacall honestly because i have not used it in a few years | 16:27 |
| dansmith | I will propose the revert for that if you don't | 16:28 |
| sean-k-mooney | aggressive = 3 is the actual change in behavior but when we did change it it dint really change mouch code | 16:29 |
| sean-k-mooney | but sure if tis cause issue swe can drop that | 16:29 |
| * sean-k-mooney signed of by.... | 16:30 | |
| sean-k-mooney | ok i need to do that form the cli... | 16:31 |
| tkajinam | or you can just copy-paste the string | 16:33 |
| tkajinam | it does not require actual signing | 16:33 |
| dansmith | Uggla: I'd really like to get wider team agreement on more things like this: https://review.opendev.org/c/openstack/nova/+/949292 before they merge | 16:33 |
| sean-k-mooney | tried that but i cant use my normal email if i do that | 16:33 |
| tkajinam | ah, yeah | 16:34 |
| sean-k-mooney | i could swap to my redhat one but it complain about the commit not matchign the sgined off by line since my redhat email is my gerrit default | 16:34 |
| sean-k-mooney | it woudl be better if it allwoed any of them or allowed my to select like if im editing things in the ui | 16:34 |
| tkajinam | I remember I faced similar problems when I tried to rebase your change. | 16:34 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Rally job for eventlet-removal https://review.opendev.org/c/openstack/nova/+/960130 | 16:34 |
| tkajinam | so yeah I agree with you | 16:35 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Rally job for eventlet-removal https://review.opendev.org/c/openstack/nova/+/960130 | 16:35 |
| opendevreview | Dan Smith proposed openstack/nova master: Fix fast8 tox target https://review.opendev.org/c/openstack/nova/+/960149 | 16:36 |
| dansmith | bauzas: ^ can haz please to fix fast8 | 16:36 |
| dansmith | it's trivial | 16:36 |
| opendevreview | sean mooney proposed openstack/nova master: Revert "Allow autopep8 to fix more things" https://review.opendev.org/c/openstack/nova/+/960150 | 16:36 |
| dansmith | bauzas: and also that ^ | 16:37 |
| sean-k-mooney | dansmith: can you test that locally by remving the agressive=3 line | 16:37 |
| sean-k-mooney | that actully the only thing it is really changing | 16:37 |
| sean-k-mooney | i think the default is 0 or 1 | 16:38 |
| dansmith | I don't ever want in-place=True, assuming that's the permission to modify anything | 16:40 |
| sean-k-mooney | yes that what that enables | 16:41 |
| dansmith | removing aggressive=3 still modifies my file | 16:41 |
| sean-k-mooney | when invoked form per-commit it will do that by defualt adn recurse | 16:41 |
| dansmith | yeah -2 on that | 16:42 |
| sean-k-mooney | well we have been doing that and agreed to do that for years | 16:42 |
| sean-k-mooney | that was not part of this change | 16:42 |
| sean-k-mooney | i jsut encoded the defaults we already had implictly | 16:42 |
| dansmith | we have not "agreed to do that for years" | 16:42 |
| sean-k-mooney | well it not new | 16:42 |
| sean-k-mooney | it was not part of that change | 16:43 |
| dansmith | I do not agree with the fact that we're using pre-commit for this stuff, and you and others sold it as "dude it's totally optional if you just don't install this" | 16:44 |
| dansmith | well this sort of modifying of my stuff while I'm working is not okay with me and it's been a nuisance this whole time but today it really blew up my whole morning | 16:45 |
| sean-k-mooney | when autopep8 was added it was entirly optional | 16:45 |
| dansmith | I do not think it's unreasonable to have a target you can run if you want to autopep8 your code | 16:45 |
| dansmith | I do not want that | 16:45 |
| dansmith | sean-k-mooney: right and now I can't even run the pep8 target (even with your revert applied) without it making changes to my code | 16:45 |
| dansmith | removing the aggressive line makes those changes much less stupid, but it still does it | 16:46 |
| sean-k-mooney | right becaus that was changed by https://github.com/openstack/nova/commit/2ffdf2e9173ca2ad0f95ac7f565da64b8b62691f | 16:46 |
| sean-k-mooney | when we swap to runign pep via per-commit we got precommits default of formatign the code by default | 16:46 |
| sean-k-mooney | this has been the case since december 2023 | 16:47 |
| dansmith | what is your point? | 16:47 |
| sean-k-mooney | 1 other want the changed behvior, 2 i didnt actully change it and 3 its not new | 16:47 |
| sean-k-mooney | so please dont shot the messanger im jsut providing you the lines to the changes that modifed the behvior | 16:48 |
| dansmith | I understand this has been modifying my code against my will since end of 2023.. I'm saying I've been hating it since then and as of this morning I've been tolerating it because the changes have been minor | 16:48 |
| dansmith | I think it's very reasonable to have pre-commit and the pep8 target not modify anything but just warn, and if people want to run autopep8 on their code, we can have a tox target to do so | 16:50 |
| sean-k-mooney | to be clear the fact this has signifcantly reduced the tiem we talk about formating has been a major quality of life improvment for me in general. we coudl revert to not using pre-commt in pep8 targe tbut that was also a sginficat performace win | 16:50 |
| dansmith | sean-k-mooney: I understand that you feel that way, but I do not | 16:51 |
| dansmith | you asserting that we as a team agreed on something you wanted when it was merged five days before christmas when maaaany people are on PTO is a major quality of life non-improvement for me | 16:52 |
| dansmith | so it'd be helpful if you could stop doing that | 16:52 |
| sean-k-mooney | the change was up for review for several months | 16:54 |
| sean-k-mooney | im not assertign that because i wanted it | 16:54 |
| sean-k-mooney | just that it wen through our normal review process | 16:54 |
| sean-k-mooney | anywya i dont think this is a useful confveration to have now but if you want to have it when more are aroudn thne do | 16:55 |
| sean-k-mooney | we can alwasy bring it up in an irc meetign or at the ptg | 16:55 |
| dansmith | gibi: I know you're pro-formatting.. can you understand that I want to be able to pre-pep8 my code before submitting without autopep8 modifying my local index, especially given this morning it was breaking it pretty badly? | 16:58 |
| dansmith | at least enough to have two targets, one of which just runs without modifying? | 16:58 |
| dansmith | if more people want the default target to run modify mode then fine, but I want to be able to run a linter in read-only mode | 16:59 |
| dansmith | it kills me that I can't git rebase -x to check without it interrupting me | 16:59 |
| sean-k-mooney | those are thigns we can likely improve and im currently testing your fast8 fix | 17:00 |
| dansmith | even if I can just `touch .do-not-fuck-up-my-code` and have that drive the in-place flag, that's fine | 17:01 |
| sean-k-mooney | i need to look into how we can do that but ill see if we can supprot somethign ideally via an env var set by tox for you | 17:02 |
| sean-k-mooney | i have never seen it actully break code in my own experince by the way so im sorry you have encotered that | 17:03 |
| sean-k-mooney | dansmith: the fast8 fix works for me locally so i have approved it | 17:06 |
| sean-k-mooney | sylvain was already +2 | 17:06 |
| gibi | dansmith: (with limited brain power left in me) you want to run pep8 locally but you dont want autopep8 to modify your code because you are in a middle of something. Still you are (not happily) accepting the fact that at the end before you push up the commit you will run autopep8 and let it do its things. If that is the case then totally supportive to your needs | 17:06 |
| gibi | * then I totally | 17:07 |
| dansmith | gibi: I do not ever want autopep8 to modify my stuff, even pre-push.. I want it to tell me what it objects to | 17:07 |
| sean-k-mooney | for what its worth pre-commit ignored unstaged files but obviously that woudl be a change in your workflow jsut lettign you know | 17:07 |
| dansmith | but yes, the major complaint is when I'm in the middle of a rebase or a complex change and it modifies my files and complicates my index so I have to manually revert things I don't want to change in the middle of a workflow | 17:08 |
| sean-k-mooney | dansmith: that how i orgianlly implented it for what its worth autopep8 in the pep8 target complaied but did nto modify | 17:08 |
| sean-k-mooney | in its sepreate target it fixed things | 17:08 |
| sean-k-mooney | and via pre-commit it fixed things | 17:08 |
| dansmith | sean-k-mooney: I know, that's what I want | 17:08 |
| dansmith | since I'm not going to use pre-commit, that was fine.. and it was presented to us as fully optional because (a) it was a different tox target or (b) pre-commit was optional if you installed it | 17:09 |
| dansmith | then, we moved to pep8 target using pre-commit and it's been complicating rebase workflows ever seince | 17:09 |
| gibi | dansmith: yeah I should have be more specific. Either running autopep8 and let it do its things, or run autopep8 and fail if wants to modify something and you iterate manually until autopep8 is happy | 17:09 |
| dansmith | gibi: yes, thanks | 17:09 |
| gibi | dansmith: I'm OK with this approach. I will rely on autopep8, you will satisfy autopep8 manually. We won't clash and in CI the end result is the same happy autopep8 run | 17:10 |
| dansmith | I had been working around this by using fast8 (which I usually use during a rebase anyway) and not complaining, but that broke recently as well hence the issue | 17:10 |
| dansmith | gibi: ++ | 17:11 |
| sean-k-mooney | im trying to think how to do that today and i htink i might know | 17:11 |
| sean-k-mooney | give me a minute to test it | 17:11 |
| sean-k-mooney | the applciation alwasy awa not actully intentional as part of stephens change. i.e. the removal of the opt out | 17:13 |
| sean-k-mooney | ill try fixign that quickly as i have 2 idea of how to do that cleanly | 17:13 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Rally job for eventlet-removal https://review.opendev.org/c/openstack/nova/+/960130 | 17:21 |
| sean-k-mooney | i need to chnage my testcase since it currently brakign the wrong hook but i think i have it working lcoally with a very minor change ill push it for review in 10 mins or so | 17:28 |
| opendevreview | Merged openstack/nova master: Reproducer for bug 2114951 https://review.opendev.org/c/openstack/nova/+/952894 | 17:48 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Rally job for eventlet-removal https://review.opendev.org/c/openstack/nova/+/960130 | 17:53 |
| opendevreview | sean mooney proposed openstack/nova master: refactor autopep8 pre-commit config https://review.opendev.org/c/openstack/nova/+/960167 | 18:11 |
| sean-k-mooney | i need to test ^ a little more but i belive it keeps the bhavior gibi and i woudl like while also restorign the behavior dan desires | 18:13 |
| sean-k-mooney | one of the things i need to confirm is that the exit code propagate form bash -c | 18:14 |
| sean-k-mooney | i belive it should but we need that to ensure that tox -e pep8 fails if any of the other check fail | 18:14 |
| sean-k-mooney | this is based on https://github.com/pre-commit/pre-commit/issues/2083 | 18:15 |
| opendevreview | Merged openstack/nova master: Update compute rpc alias for epoxy https://review.opendev.org/c/openstack/nova/+/959181 | 19:17 |
| opendevreview | Merged openstack/nova master: Fix fast8 tox target https://review.opendev.org/c/openstack/nova/+/960149 | 19:43 |
| opendevreview | Tobias Urdin proposed openstack/nova master: libvirt: update description for live_migration_completion_timeout https://review.opendev.org/c/openstack/nova/+/874083 | 20:36 |
| opendevreview | sean mooney proposed openstack/nova master: refactor autopep8 pre-commit config https://review.opendev.org/c/openstack/nova/+/960167 | 21:22 |
Generated by irclog2html.py 4.0.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!