| opendevreview | Ho Minh Quang Ngo proposed openstack/watcher master: Add options to disable migration in host maintenance https://review.opendev.org/c/openstack/watcher/+/952538 | 04:19 |
|---|---|---|
| opendevreview | chandan kumar proposed openstack/watcher master: replace autopep8 with ruff. https://review.opendev.org/c/openstack/watcher/+/948034 | 05:07 |
| opendevreview | chandan kumar proposed openstack/watcher master: replace autopep8 with ruff. https://review.opendev.org/c/openstack/watcher/+/948034 | 05:33 |
| opendevreview | chandan kumar proposed openstack/watcher master: replace autopep8 with ruff. https://review.opendev.org/c/openstack/watcher/+/948034 | 06:15 |
| opendevreview | chandan kumar proposed openstack/watcher master: Drops forbidden patch/delete/post action apis https://review.opendev.org/c/openstack/watcher/+/954021 | 06:21 |
| opendevreview | Douglas Viroel proposed openstack/watcher master: WIP - Extend decision engine to support threading mode https://review.opendev.org/c/openstack/watcher/+/952257 | 12:18 |
| opendevreview | chandan kumar proposed openstack/watcher master: Drop Code related to OperationNotPermitted exception https://review.opendev.org/c/openstack/watcher/+/954120 | 12:46 |
| chandankumar | sean-k-mooney: dviroel Hello, based on yesterday discussion around OperationNotPermitted exception, I have added following bug https://launchpad.net/bugs/2115968 and code removal cr https://review.opendev.org/c/openstack/watcher/+/954120 | 13:13 |
| chandankumar | this exception was introduced during the initial import of watcher code | 13:14 |
| chandankumar | the only place I am worried is here https://github.com/openstack/watcher/blob/master/watcher/api/controllers/v1/audit_template.py#L211 | 13:14 |
| chandankumar | I tried to replace that piece with Not Authorized exception to keep the http status code same. | 13:15 |
| chandankumar | I am not sure about only that piece but rest of the stuff is straight removal. | 13:15 |
| chandankumar | feel free to leave your thoughts on that on the review itself, thank you! | 13:16 |
| sean-k-mooney | chandankumar: ack i was thinkign fo doing that myslef this spint so cool ill add that to my review list | 13:22 |
| sean-k-mooney | i started lookign at the code a bit after the meeting and i did not see any places where the class varibale was ever beign set to true | 13:22 |
| sean-k-mooney | i assuem you didnt find any either? | 13:23 |
| chandankumar | yes correct! | 13:23 |
| sean-k-mooney | so for https://github.com/openstack/watcher/blob/master/watcher/api/controllers/v1/audit_template.py#L211 | 13:23 |
| sean-k-mooney | unauthorized is not the corret responce | 13:23 |
| sean-k-mooney | it shoudl be a 400 bad reuqest | 13:23 |
| sean-k-mooney | you coudl arguee for a 409 conflict but its not a permission issue | 13:24 |
| sean-k-mooney | i think 400 bad request is more semanticly correct in terms fo respocne codes | 13:24 |
| sean-k-mooney | chandankumar: woudl you mind updating https://review.opendev.org/c/openstack/watcher/+/954120/1/watcher/api/controllers/v1/audit_template.py#211 to reflect that | 13:25 |
| chandankumar | if we go towards 400 code, then wsme.exec.ClientSideError will take care of that | 13:25 |
| sean-k-mooney | yep that should be fine | 13:25 |
| chandankumar | let me update that | 13:26 |
| sean-k-mooney | skimming the rest is looks ok to me | 13:26 |
| sean-k-mooney | obviously i want ot see the ci results but the only other suggestion i have is we coudl add a fixes release note for this but its not strictly requried in this case | 13:27 |
| chandankumar | Done, Added releasenote also | 13:37 |
| sean-k-mooney | cool ill try an check back later once ci has run can you assign https://bugs.launchpad.net/watcher/+bug/2115968 to your self and set the imporant to low | 13:38 |
| chandankumar | sure, thank you! | 13:40 |
| sean-k-mooney | chandankumar: by the way i dont knwo if you saw my stable reviews or not | 13:40 |
| chandankumar | sean-k-mooney: I think alfredo updated some of the backports. there are few comments, I will go over them, thank you for the reminder. | 13:41 |
| sean-k-mooney | can you make sure that if your doing a stabel backport that you alwsy set the topic branch back to bug/#### without the -stable suffix and make sure that you properly cherry pick each patch on top of the prior one. | 13:41 |
| sean-k-mooney | ack | 13:41 |
| sean-k-mooney | most of the issue were related to haveign each patch against the target bacnh instead of properly stackign them | 13:42 |
| sean-k-mooney | we want them ot be stack to test them togher, and we want to use the same topic as the master change which shoudl be bug/<number> so that the are properly grouped | 13:42 |
| sean-k-mooney | it makes it much much simpler to review when we follwo those convenstions | 13:43 |
| opendevreview | chandan kumar proposed openstack/watcher master: Drop Code related to OperationNotPermitted exception https://review.opendev.org/c/openstack/watcher/+/954120 | 14:01 |
| chandankumar | we should avoid any backport, does not having bug linked with it to stable branches. | 14:03 |
| chandankumar | Let me fix them | 14:03 |
| dviroel | chandankumar: ack, thanks | 14:12 |
Generated by irclog2html.py 4.0.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!