Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2025-09-08
16:37:39 sean-k-mooney dansmith: can you test that locally by remving the agressive=3 line
16:37:50 sean-k-mooney that actully the only thing it is really changing
16:38:07 sean-k-mooney i think the default is 0 or 1
16:40:54 dansmith I don't ever want in-place=True, assuming that's the permission to modify anything
16:41:19 sean-k-mooney yes that what that enables
16:41:55 sean-k-mooney when invoked form per-commit it will do that by defualt adn recurse
16:41:55 dansmith removing aggressive=3 still modifies my file
16:42:04 dansmith yeah -2 on that
16:42:19 sean-k-mooney well we have been doing that and agreed to do that for years
16:42:24 sean-k-mooney that was not part of this change
16:42:36 sean-k-mooney i jsut encoded the defaults we already had implictly
16:42:48 dansmith we have not "agreed to do that for years"
16:42:58 sean-k-mooney well it not new
16:43:10 sean-k-mooney it was not part of that change
16:44:35 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:45:06 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:17 sean-k-mooney when autopep8 was added it was entirly optional
16:45:22 dansmith I do not think it's unreasonable to have a target you can run if you want to autopep8 your code
16:45:38 dansmith I do not want that
16:45:56 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:46:08 dansmith removing the aggressive line makes those changes much less stupid, but it still does it
16:46:09 sean-k-mooney right becaus that was changed by https://github.com/openstack/nova/commit/2ffdf2e9173ca2ad0f95ac7f565da64b8b62691f
16:46:53 sean-k-mooney when we swap to runign pep via per-commit we got precommits default of formatign the code by default
16:47:12 sean-k-mooney this has been the case since december 2023
16:47:24 dansmith what is your point?
16:47:47 sean-k-mooney 1 other want the changed behvior, 2 i didnt actully change it and 3 its not new
16:48:34 sean-k-mooney so please dont shot the messanger im jsut providing you the lines to the changes that modifed the behvior
16:48:35 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:50:46 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:49 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:51:19 dansmith sean-k-mooney: I understand that you feel that way, but I do not
16:52:19 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:25 dansmith so it'd be helpful if you could stop doing that
16:54:18 sean-k-mooney the change was up for review for several months
16:54:42 sean-k-mooney im not assertign that because i wanted it
16:54:51 sean-k-mooney just that it wen through our normal review process
16:55:40 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:56 sean-k-mooney we can alwasy bring it up in an irc meetign or at the ptg
16:58:16 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:29 dansmith at least enough to have two targets, one of which just runs without modifying?
16:59:01 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:23 dansmith it kills me that I can't git rebase -x to check without it interrupting me
17:00:04 sean-k-mooney those are thigns we can likely improve and im currently testing your fast8 fix
17:01:12 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:02:10 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:03:20 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:06:05 sean-k-mooney dansmith: the fast8 fix works for me locally so i have approved it
17:06:32 sean-k-mooney sylvain was already +2
17:06:37 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:07:11 gibi * then I totally
17:07:35 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:47 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:08:04 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:36 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:42 sean-k-mooney in its sepreate target it fixed things
17:08:48 sean-k-mooney and via pre-commit it fixed things
17:08:50 dansmith sean-k-mooney: I know, that's what I want
17:09:22 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:41 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:41 dansmith then, we moved to pep8 target using pre-commit and it's been complicating rebase workflows ever seince
17:09:53 dansmith gibi: yes, thanks
17:10:41 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:58 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:11:08 dansmith gibi: ++
17:11:16 sean-k-mooney im trying to think how to do that today and i htink i might know
17:11:33 sean-k-mooney give me a minute to test it
17:13:14 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:36 sean-k-mooney ill try fixign that quickly as i have 2 idea of how to do that cleanly
17:21:23 opendevreview Balazs Gibizer proposed openstack/nova master: Rally job for eventlet-removal https://review.opendev.org/c/openstack/nova/+/960130
17:28:37 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:48:30 opendevreview Merged openstack/nova master: Reproducer for bug 2114951 https://review.opendev.org/c/openstack/nova/+/952894
17:53:10 opendevreview Balazs Gibizer proposed openstack/nova master: Rally job for eventlet-removal https://review.opendev.org/c/openstack/nova/+/960130
18:11:17 opendevreview sean mooney proposed openstack/nova master: refactor autopep8 pre-commit config https://review.opendev.org/c/openstack/nova/+/960167
18:13:05 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:14:20 sean-k-mooney one of the things i need to confirm is that the exit code propagate form bash -c
18:14:54 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:15:53 sean-k-mooney this is based on https://github.com/pre-commit/pre-commit/issues/2083
19:17:48 opendevreview Merged openstack/nova master: Update compute rpc alias for epoxy https://review.opendev.org/c/openstack/nova/+/959181
19:43:34 opendevreview Merged openstack/nova master: Fix fast8 tox target https://review.opendev.org/c/openstack/nova/+/960149
20:36:18 opendevreview Tobias Urdin proposed openstack/nova master: libvirt: update description for live_migration_completion_timeout https://review.opendev.org/c/openstack/nova/+/874083
21:22:14 opendevreview sean mooney proposed openstack/nova master: refactor autopep8 pre-commit config https://review.opendev.org/c/openstack/nova/+/960167
#openstack-nova - 2025-09-09
09:07:26 opendevreview Balazs Gibizer proposed openstack/nova stable/2025.1: Reproduce bug/2115905 https://review.opendev.org/c/openstack/nova/+/960241
09:07:27 opendevreview Balazs Gibizer proposed openstack/nova stable/2025.1: [PCI tracker]Remove non configured devs when freed https://review.opendev.org/c/openstack/nova/+/960242
09:07:28 opendevreview Balazs Gibizer proposed openstack/nova stable/2025.1: [pci]Keep used dev in Placement regardless of dev_spec https://review.opendev.org/c/openstack/nova/+/960243
09:14:02 opendevreview Balazs Gibizer proposed openstack/nova stable/2024.2: Reproduce bug/2115905 https://review.opendev.org/c/openstack/nova/+/960244
09:14:03 opendevreview Balazs Gibizer proposed openstack/nova stable/2024.2: [PCI tracker]Remove non configured devs when freed https://review.opendev.org/c/openstack/nova/+/960245
09:14:04 opendevreview Balazs Gibizer proposed openstack/nova stable/2024.2: [pci]Keep used dev in Placement regardless of dev_spec https://review.opendev.org/c/openstack/nova/+/960246
09:21:27 opendevreview Balazs Gibizer proposed openstack/nova stable/2024.1: Reproduce bug/2115905 https://review.opendev.org/c/openstack/nova/+/960248
09:21:28 opendevreview Balazs Gibizer proposed openstack/nova stable/2024.1: [PCI tracker]Remove non configured devs when freed https://review.opendev.org/c/openstack/nova/+/960249
09:21:29 opendevreview Balazs Gibizer proposed openstack/nova stable/2024.1: [pci]Keep used dev in Placement regardless of dev_spec https://review.opendev.org/c/openstack/nova/+/960250
09:41:04 opendevreview Johannes Kulik proposed openstack/nova master: api: Pre-query not deleted members in server groups https://review.opendev.org/c/openstack/nova/+/959696
13:30:54 Uggla dansmith, I have seen you point above, would you like to "quickly" discuss about it at PTG ?
13:38:16 dansmith Uggla: I would like it fixed right now (i.e. stop forcing edits to my code) and yes, a quick discussion to get agreement that there should always be _some_ way to run the linters in read-only mode might be good
14:01:34 opendevreview sean mooney proposed openstack/nova master: remove hardcoded in-place config for autopep8 https://review.opendev.org/c/openstack/nova/+/960150
14:01:35 opendevreview sean mooney proposed openstack/nova master: refactor autopep8 pre-commit config https://review.opendev.org/c/openstack/nova/+/960167
14:22:09 opendevreview Balazs Gibizer proposed openstack/nova master: Rally job for eventlet-removal https://review.opendev.org/c/openstack/nova/+/960130
14:26:09 opendevreview Balazs Gibizer proposed openstack/nova master: Rally job for eventlet-removal https://review.opendev.org/c/openstack/nova/+/960130
14:51:50 opendevreview sean mooney proposed openstack/nova master: Revert "Allow autopep8 to fix more things" https://review.opendev.org/c/openstack/nova/+/960150
14:51:51 opendevreview sean mooney proposed openstack/nova master: renable autopep8 agressive=3 https://review.opendev.org/c/openstack/nova/+/960274
14:51:51 opendevreview sean mooney proposed openstack/nova master: refactor autopep8 pre-commit config https://review.opendev.org/c/openstack/nova/+/960167

Earlier   Later