| *** mhen_ is now known as mhen | 01:17 | |
| croelandt | Reminder that we must merge https://review.opendev.org/q/topic:%22bug/2110185%22 today if we want it to be part of M2 :) | 12:25 |
|---|---|---|
| croelandt | dansmith: ^ | 12:25 |
| croelandt | rosmaita: ^ | 12:25 |
| dansmith | croelandt: ack, was out friday, but I see there were changes | 13:54 |
| dansmith | abhishek_: around? | 14:01 |
| abhishek_ | dansmith: yes | 14:05 |
| dansmith | abhishek_: on the s3 patch, several things seem ack'd but not changed or replied, I'm wondering if some comments got dropped? | 14:05 |
| abhishek_ | let me check | 14:06 |
| dansmith | https://review.opendev.org/c/openstack/glance_store/+/949129/6..7/glance_store/_drivers/s3.py L667 ... why is image_size optional? it is only ever called from this file so we can just pass it as positional right? | 14:06 |
| abhishek_ | ohh, I misread that | 14:08 |
| dansmith | the name change is good though | 14:08 |
| dansmith | also, | 14:08 |
| abhishek_ | sorry, I was under impression that keep the param name same as multipart_upload | 14:08 |
| dansmith | the param name is good, just no need to make it optional, especially defaulted to something we don't want | 14:09 |
| abhishek_ | ack, I will work on that but will take some time as I need to step away atm :( | 14:10 |
| dansmith | also, in the tests, try..except..assertIn is an anti-pattern because the wrong exception will cause the test to explode instead of report the exception | 14:10 |
| dansmith | assertRaisesRegex() is the standard | 14:10 |
| dansmith | abhishek_: ack, I will fix then | 14:10 |
| abhishek_ | sorry for the trouble :/ | 14:11 |
| abhishek_ | I can send followup patch for changing assertIn to assertRaisesRagex | 14:13 |
| dansmith | ahh, I see.. because we pass all the other positionals as keyword (which we shouldn't) is probably why | 14:14 |
| dansmith | still don't need to | 14:15 |
| abhishek_ | ack | 14:16 |
| * abhishek_ sorry, I need to step out, will be back in 3-4 hours | 14:21 | |
| dansmith | argh, all the duplication in these tests means soooo much repeat fixing | 14:22 |
| opendevreview | Dan Smith proposed openstack/glance_store master: [s3] Add size validation for image uploads https://review.opendev.org/c/openstack/glance_store/+/949129 | 14:40 |
| opendevreview | Dan Smith proposed openstack/glance_store master: [filesystem] Add size validation for image uploads https://review.opendev.org/c/openstack/glance_store/+/949124 | 14:40 |
| opendevreview | Dan Smith proposed openstack/glance_store master: [swift] Add size validation for image uploads https://review.opendev.org/c/openstack/glance_store/+/949626 | 14:40 |
| opendevreview | Dan Smith proposed openstack/glance_store master: [rbd] Add size validation for image uploads https://review.opendev.org/c/openstack/glance_store/+/949362 | 14:40 |
| opendevreview | Dan Smith proposed openstack/glance_store master: [cinder] Add size validation for image uploads https://review.opendev.org/c/openstack/glance_store/+/949137 | 14:40 |
| dansmith | croelandt: I fixed the bottom one but am realizing the exception assertion runs through the whole stack, so I'll just have to review those on the grounds that they're going to get fixed later :/ | 14:40 |
| dansmith | croelandt: I'm +2 on the stack with followup reminders | 14:54 |
| croelandt | OK I guess I'll go have another look at those patches, we'll merge them and expect a followup fix | 15:08 |
| croelandt | dansmith: ^ | 15:08 |
| dansmith | yep | 15:09 |
| croelandt | https://review.opendev.org/c/openstack/glance_store/+/949124/9..11/glance_store/tests/unit/test_multistore_filesystem.py damn we went backwards here | 15:38 |
| croelandt | oh in Swift as well | 15:39 |
| * croelandt is confused | 15:39 | |
| dansmith | yep, I commented on a bunch of them and we discussed above | 15:51 |
| opendevreview | Takashi Kajinami proposed openstack/glance_store master: Replace os-client-config https://review.opendev.org/c/openstack/glance_store/+/953796 | 15:53 |
| opendevreview | Takashi Kajinami proposed openstack/glance_store master: Replace os-client-config https://review.opendev.org/c/openstack/glance_store/+/953796 | 15:56 |
| dansmith | croelandt: I think we actually have until Jul 3 for this right? | 15:57 |
| opendevreview | Takashi Kajinami proposed openstack/glance_store master: Replace os-client-config https://review.opendev.org/c/openstack/glance_store/+/953796 | 15:57 |
| dansmith | schedule shows this whole week is M2 | 15:57 |
| opendevreview | Takashi Kajinami proposed openstack/python-glanceclient master: Replace os-client-config https://review.opendev.org/c/openstack/python-glanceclient/+/953799 | 15:58 |
| croelandt | dansmith: M2 is Jul 3, but I need to send the patch, and we need to get everything merged before then | 16:01 |
| croelandt | I'm trying not to wait for the last minute here :) | 16:02 |
| opendevreview | Takashi Kajinami proposed openstack/glance_store master: Replace os-client-config https://review.opendev.org/c/openstack/glance_store/+/953796 | 16:04 |
| opendevreview | Takashi Kajinami proposed openstack/python-glanceclient master: Replace os-client-config https://review.opendev.org/c/openstack/python-glanceclient/+/953799 | 16:05 |
| dansmith | croelandt: ack, setting the deadline arbitrarily early for behavioral reasons is all good, just pointing out that you technically have more time.... | 16:05 |
| croelandt | yep | 16:06 |
| croelandt | should we use that time for something in particular? | 16:06 |
| dansmith | I just mean if abhi shows up a and re-re-factors the assertRaisesRegex stuff, we could potentially shove that in and still make the deadline | 16:10 |
| dansmith | but otherwise, no, I was just noting the actual deadline "week" | 16:10 |
| croelandt | yeah I'm hoping to shove the Regex stuff in | 16:11 |
| dansmith | cool | 16:14 |
| opendevreview | Takashi Kajinami proposed openstack/glance_store master: Replace os-client-config https://review.opendev.org/c/openstack/glance_store/+/953796 | 16:15 |
| opendevreview | Takashi Kajinami proposed openstack/python-glanceclient master: Replace os-client-config https://review.opendev.org/c/openstack/python-glanceclient/+/953799 | 16:15 |
| croelandt | tkajinam: what's going on here? :) | 16:17 |
| tkajinam | sorry for the noise. I messed up the fix while I'm distributing it... | 16:22 |
| tkajinam | was trying to push updates after seeing https://review.opendev.org/c/openstack/os-client-config/+/953790 | 16:22 |
| tkajinam | before it drops off form my memory... | 16:22 |
| croelandt | ok ok | 16:33 |
| croelandt | tkajinam: if you fix the typos from https://review.opendev.org/c/openstack/glance_store/+/932251 I will +2 and I think Abhishek will be around again in a few hours | 16:33 |
| croelandt | so we can probably have this merged tonight | 16:33 |
| opendevreview | Takashi Kajinami proposed openstack/glance_store master: swift: Drop support for v1/v2 auth https://review.opendev.org/c/openstack/glance_store/+/932251 | 16:34 |
| tkajinam | croelandt, thanks ! | 16:34 |
| croelandt | so quick | 16:35 |
| tkajinam | :-) | 16:36 |
| opendevreview | Merged openstack/glance_store master: Add support for backend_defaults group https://review.opendev.org/c/openstack/glance_store/+/946512 | 18:05 |
| opendevreview | Merged openstack/glance_store master: [s3] Add size validation for image uploads https://review.opendev.org/c/openstack/glance_store/+/949129 | 20:42 |
| opendevreview | Merged openstack/glance_store master: [filesystem] Add size validation for image uploads https://review.opendev.org/c/openstack/glance_store/+/949124 | 21:08 |
| opendevreview | Merged openstack/glance_store master: [swift] Add size validation for image uploads https://review.opendev.org/c/openstack/glance_store/+/949626 | 21:28 |
| opendevreview | Merged openstack/glance_store master: [rbd] Add size validation for image uploads https://review.opendev.org/c/openstack/glance_store/+/949362 | 21:28 |
| opendevreview | Merged openstack/glance_store master: [cinder] Add size validation for image uploads https://review.opendev.org/c/openstack/glance_store/+/949137 | 21:28 |
Generated by irclog2html.py 4.0.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!