| opendevreview | Merged openstack/glance_store master: Doc: Use Block Storage API v3 https://review.opendev.org/c/openstack/glance_store/+/802153 | 05:34 |
|---|---|---|
| *** bhagyashris_ is now known as bhagyashris | 05:39 | |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Move metadef object policy checks in the API https://review.opendev.org/c/openstack/glance/+/799634 | 06:07 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Move metadef resource type association policy checks in the API https://review.opendev.org/c/openstack/glance/+/799637 | 06:58 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Move metadef property policy checks in the API https://review.opendev.org/c/openstack/glance/+/799635 | 07:18 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Move metadef tag policy checks in the API https://review.opendev.org/c/openstack/glance/+/799636 | 07:58 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Refactor gateway auth layer for member APIs https://review.opendev.org/c/openstack/glance/+/802525 | 09:11 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Move member policy checks to API layer https://review.opendev.org/c/openstack/glance/+/802996 | 09:11 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Refactor gateway auth layer for task APIs https://review.opendev.org/c/openstack/glance/+/802243 | 09:28 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Deprecate task specific policies https://review.opendev.org/c/openstack/glance/+/802244 | 09:28 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Move Tasks policy checks in the API https://review.opendev.org/c/openstack/glance/+/802245 | 09:28 |
| opendevreview | Merged openstack/glance_store master: swift: Take into account swift_store_endpoint https://review.opendev.org/c/openstack/glance_store/+/776611 | 10:11 |
| *** abhishekk is now known as akekane|home | 13:44 | |
| *** akekane|home is now known as abhishekk | 13:44 | |
| abhishekk | @all cores, I have 10 patches which are in nice shape (hopefully), kindly review | 13:48 |
| abhishekk | https://review.opendev.org/q/topic:%2522policy-refactor%2522+(status:open) | 13:49 |
| dansmith | abhishekk: This https://review.opendev.org/c/openstack/glance/+/799634/17/glance/tests/functional/v2/test_metadef_object_api_policy.py line 209... | 14:00 |
| abhishekk | looking | 14:00 |
| dansmith | that does what we want, but I'm not sure why.. where does get_metadef_namespace get checked if we got the repo with auth_layer=False ? | 14:01 |
| abhishekk | dansmith, same will be case for delete as well | 14:04 |
| dansmith | right, but you're saying that disabling get_metadef_namespace policy will cause the check on L192 here https://review.opendev.org/c/openstack/glance/+/799634/17/glance/api/v2/metadef_objects.py to fail right? | 14:04 |
| abhishekk | https://review.opendev.org/c/openstack/glance/+/799634/17/glance/api/v2/policy.py | 14:06 |
| abhishekk | line #178 | 14:06 |
| dansmith | ah, right okay, so your get is not failing (which means no test coverage for L67 in metadef_objects.py), you just get the repo to pass to the policy check, which then checks get_ if the update_ check fails | 14:08 |
| dansmith | so can we add a test to make sure that if the get fails we get the right notfound as well? | 14:08 |
| dansmith | L67 here: https://review.opendev.org/c/openstack/glance/+/799634/17/glance/api/v2/metadef_objects.py | 14:08 |
| abhishekk | looking | 14:09 |
| abhishekk | ack | 14:10 |
| dansmith | do you want to do that in a separate patch or just rev this real quick? | 14:10 |
| dansmith | unless someone is going to +W this right now, it's probably quicker to rev this, but that's my only concern and can +2 if you're going to do it separately | 14:10 |
| abhishekk | I think I will do it in same patch, and need to replicate the same in other patches as well | 14:11 |
| dansmith | okay | 14:11 |
| abhishekk | Comment on the patch and vote -1 there | 14:11 |
| dansmith | okay | 14:11 |
| abhishekk | also I think same is needed in namespace patch as well | 14:11 |
| abhishekk | You can then shift focus to tasks and members patches :D | 14:12 |
| abhishekk | But other than that its good right? | 14:13 |
| dansmith | yup! | 14:13 |
| abhishekk | cool | 14:15 |
| abhishekk | will add your comment as a note as well | 14:16 |
| dansmith | ack | 14:16 |
| abhishekk | Note to other reviewers, this causes our "check get if modify fails" logic to return 404 as we expect, but not related to the latest rev that checks the namespace get operation first. | 14:16 |
| abhishekk | this one | 14:16 |
| dansmith | cool | 14:18 |
| dansmith | abhishekk: This is exactly the same pattern as several others, think I can probably ninja this unless you'd rather get someone else to look: https://review.opendev.org/c/openstack/glance/+/802525 | 14:18 |
| abhishekk | croelandt, has already given +2 on previous patch and after that I have just rebased it, so I think good to go | 14:19 |
| dansmith | ah yeah, cool | 14:20 |
| abhishekk | dansmith, just confirming one more time, new test needed for getting non-existing namespace which will return 404 right | 14:26 |
| dansmith | not non-existing, because that will raise on its own, | 14:27 |
| dansmith | but you catch forbidden from ns_repo.get() and translate to NotFound, but you don't test that (AFAICT) | 14:27 |
| abhishekk | ack | 14:29 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Move metadef namespace policy checks in the API https://review.opendev.org/c/openstack/glance/+/799633 | 15:20 |
| abhishekk | dansmith, please have a look at test (sorry to distract you) | 15:21 |
| opendevreview | Merged openstack/glance master: Refactor gateway auth layer for member APIs https://review.opendev.org/c/openstack/glance/+/802525 | 15:37 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Move metadef object policy checks in the API https://review.opendev.org/c/openstack/glance/+/799634 | 15:51 |
| * abhishekk going for dinner | 15:51 | |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Move metadef resource type association policy checks in the API https://review.opendev.org/c/openstack/glance/+/799637 | 16:06 |
| * abhishekk now going for dinner | 16:06 | |
| * abhishekk back | 16:43 | |
| abhishekk | dansmith, do you have any specific questions on member related stuff ? | 16:48 |
| dansmith | abhishekk: just the ones I left in the patch | 16:49 |
| dansmith | trying to finish the top metadef one now | 16:49 |
| abhishekk | ack, added answer their, take your time | 16:50 |
| abhishekk | croelandt, if around could you please have a look as well ? | 16:51 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Move metadef property policy checks in the API https://review.opendev.org/c/openstack/glance/+/799635 | 16:51 |
| * dansmith is so sick of metadef | 17:02 | |
| abhishekk | welcome to the club | 17:08 |
| dansmith | I don't like your club and would like to leave. | 17:09 |
| abhishekk | we should leave together :P | 17:10 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Move metadef tag policy checks in the API https://review.opendev.org/c/openstack/glance/+/799636 | 17:10 |
| dansmith | Union of Concerned Developers for the Removal of Metadefs | 17:11 |
| abhishekk | haha | 17:12 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Refactor gateway auth layer for task APIs https://review.opendev.org/c/openstack/glance/+/802243 | 17:42 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Deprecate task specific policies https://review.opendev.org/c/openstack/glance/+/802244 | 17:42 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Move Tasks policy checks in the API https://review.opendev.org/c/openstack/glance/+/802245 | 17:42 |
| croelandt | no one uses metadefs anyway | 18:01 |
| croelandt | we realized that when abhishekk saw that commands were missing in the client :D | 18:01 |
| abhishekk | there are more problems in it than I have highlighted :D | 18:02 |
| abhishekk | this is one more example, https://bugs.launchpad.net/glance/+bug/1939169 | 18:03 |
| croelandt | I say we remove the feature and see if anyone complains | 18:05 |
| abhishekk | :D | 18:07 |
| croelandt | What's the performance cost of creating a MetadefAPIPolicy object? | 18:11 |
| abhishekk | I don;t think it will hamper much | 18:15 |
| croelandt | yeah | 18:16 |
| croelandt | we could have avoided a few object creations in https://review.opendev.org/c/openstack/glance/+/799633/ but well, it's not our most pressing issue right now :D | 18:16 |
| abhishekk | but I see what you trying to point out | 18:16 |
| abhishekk | Add a comment there | 18:16 |
| abhishekk | I think I will push a final revision for all patches by tomorrow | 18:17 |
| croelandt | oh if you think you're gonna need one more patchset then | 18:18 |
| abhishekk | I think its better to get it right rather than pushing followups | 18:19 |
| croelandt | Do we have a list of reviews we want to merge before M3? I think you had a giant spreadsheet | 18:21 |
| * croelandt is trying to figure out when to go on PTO | 18:21 | |
| abhishekk | yes, that spreadsheet contains all important patches | 18:21 |
| abhishekk | Or we (probably someone other than me) could create custom review dashboard | 18:22 |
| croelandt | do you have the link to the spreadsheet? | 18:23 |
| abhishekk | yep | 18:25 |
| abhishekk | https://docs.google.com/spreadsheets/d/1SWBq0CsHw8jofHxmOG8QeZEX6veDE4eU0QHItOu8uQs/edit?pli=1#gid=0 | 18:25 |
| abhishekk | I think we need a plan here | 18:26 |
| abhishekk | this week we should get metadef stuff merged (or till monday as Friday will be holiday for most of all) | 18:27 |
| abhishekk | Then next week we should target members/tasks and remaining open image related actions | 18:27 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Move metadef namespace policy checks in the API https://review.opendev.org/c/openstack/glance/+/799633 | 19:03 |
| abhishekk | dansmith, croelandt I think I have addressed all comments here | 19:03 |
| dansmith | abhishekk: just replied here: https://review.opendev.org/c/openstack/glance/+/799637 | 19:04 |
| abhishekk | dansmith, ack, going to revert it | 19:05 |
| dansmith | okay, also failed zuul.. unrelated? | 19:05 |
| dansmith | yeah, cinder dance :/ | 19:06 |
| abhishekk | yes | 19:06 |
| abhishekk | purposely not added recheck as going to submit new revision | 19:07 |
| dansmith | okay | 19:07 |
| abhishekk | dansmith, do you think I should remove nested try from here as well ? | 19:09 |
| abhishekk | https://review.opendev.org/c/openstack/glance/+/799634/18/glance/api/v2/metadef_objects.py | 19:09 |
| abhishekk | I am ok to restructure it | 19:09 |
| dansmith | no, unless there's a problem let's just move on | 19:09 |
| abhishekk | no problem, but I think we should keep consistent pattern everywhere | 19:11 |
| dansmith | yup .. up to you | 19:11 |
| dansmith | restructure later would be easier just to avoid the churn in this patch.. it has changed so much that delta between PS N and M is a lot | 19:12 |
| abhishekk | ack, thanks | 19:12 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Move metadef object policy checks in the API https://review.opendev.org/c/openstack/glance/+/799634 | 19:37 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Move metadef resource type association policy checks in the API https://review.opendev.org/c/openstack/glance/+/799637 | 19:37 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Move metadef property policy checks in the API https://review.opendev.org/c/openstack/glance/+/799635 | 19:48 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Move metadef tag policy checks in the API https://review.opendev.org/c/openstack/glance/+/799636 | 20:02 |
| abhishekk | Glance Xena 3 review dashboard - https://tinyurl.com/glance-xena-3 | 20:37 |
| abhishekk | croelandt, ^^ | 20:37 |
| * abhishekk signing out for the day | 20:37 | |
| opendevreview | Merged openstack/glance master: Move metadef namespace policy checks in the API https://review.opendev.org/c/openstack/glance/+/799633 | 22:13 |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!