| *** mhen_ is now known as mhen | 03:04 | |
| mhen | hi, I'm trying to address a review comment from dansmith here: https://review.opendev.org/c/openstack/glance/+/926295/comment/41e60bfe_f69dd274/ | 13:57 |
|---|---|---|
| mhen | when I move my checks to reject encrypted images in the conversion import plugin to `get_flow()`, any exception thrown there will lead to the import plugin to silently fail | 13:58 |
| mhen | it will only print in the Glance logs that it "Could not load image_conversion" and proceed with importing without conversion | 13:58 |
| mhen | i.e. the image gets imported as-is and circumvents conversion | 13:59 |
| mhen | would that be acceptable behavior from Glance point of view or should I keep failing in the actual conversion process with the image not reaching 'active' state? | 13:59 |
| womax | hi | 14:00 |
| womax | rosmaita: thanks for your comments on the spec. Points you address we didn't have in mind at all. I'll try to work on it this week and come back to answers to your comments | 14:04 |
| rosmaita | womax: sounds good ... which spec is this? (i can't keep my reviews straight) | 14:06 |
| womax | rosmaita: this one https://review.opendev.org/c/openstack/glance-specs/+/970214 | 14:28 |
| rosmaita | womax: ok, will be interested to see what you come up with | 14:31 |
| mhen | I can't really tell from the documentation: | 15:46 |
| mhen | https://docs.openstack.org/glance/latest/admin/interoperable-image-import.html#the-image-conversion | 15:46 |
| mhen | is the conversion plugin intended to be on a best-effort basis, i.e., importing the image in its original format anyway if conversion flow fails? | 15:47 |
| mhen | or is it intended to assure that the image is converted and rejected otherwise? | 15:48 |
| croelandt | If qemu-img convert fails, you should get a RuntimeError | 15:59 |
| tkajinam | anyone care to merge https://review.opendev.org/c/openstack/python-glanceclient/+/953799 before the holiday ? ;-) | 16:26 |
| tkajinam | this is almost the last one remaining | 16:26 |
| croelandt | tkajinam: done | 16:36 |
| croelandt | so os-client-config is still a thing though | 16:36 |
| croelandt | https://review.opendev.org/c/openstack/os-client-config/+/549307 was 7 years ago so I guess os-client-config also has other use cases since it's still around? | 16:36 |
| tkajinam | croelandt, thanks | 16:40 |
| tkajinam | croelandt, os-client-config was merged into osc-lib | 16:40 |
| tkajinam | oops. not osc-lib but openstacksdk | 16:41 |
| tkajinam | croelandt, see https://review.opendev.org/c/openstack/os-client-config/+/953790/2/os_client_config/__init__.py | 16:42 |
| croelandt | damn, but it's still been receiving patches for all these years | 16:46 |
| croelandt | oh no this patch is from July | 16:46 |
| croelandt | ok ok | 16:46 |
| tkajinam | croelandt, yup. the warning was added to actually kill it. | 16:46 |
| tkajinam | this is the last minute warning before actual retirement, pending for long | 16:46 |
| croelandt | oh they want to retire it in G? | 16:47 |
| tkajinam | https://review.opendev.org/c/openstack/governance/+/953549 | 16:47 |
| tkajinam | likely but I'm not sure about the actual retirement timing. | 16:48 |
| croelandt | Interesting | 16:49 |
| tkajinam | https://review.opendev.org/c/openstack/python-glanceclient/+/959847 | 16:51 |
| tkajinam | https://review.opendev.org/c/openstack/python-glanceclient/+/965698 | 16:52 |
| tkajinam | https://review.opendev.org/c/openstack/python-glanceclient/+/936134 | 16:52 |
| tkajinam | croelandt, if you have a few more minutes do you mind throwing these to gate ? ^^^ | 16:52 |
| mhen | what is the proper etiquette on patchset comments? If a reviewer asks for a change and I implement something I think fully addresses the comment, am I allowed to mark this as resolved myself or do I wait for the reviewer to check again? | 17:15 |
| tkajinam | mhen, I'm not aware of any documented guideline or general agreement, but my own practice is marking comments done based on my own confidence. | 17:18 |
| tkajinam | and keep comments open to ask for additional actions from reviewers. | 17:20 |
| mhen | tkajinam: alright, thank you | 17:24 |
| opendevreview | Merged openstack/python-glanceclient master: Replace os-client-config https://review.opendev.org/c/openstack/python-glanceclient/+/953799 | 18:09 |
| croelandt | mhen: same, I use this as a todolist and click "Resolved" to let the reviewer know I did what they wanted | 18:24 |
| croelandt | If I go back to a patchset and don't see "resolved" under my own comments, I worry that the author has not read what I wrote | 18:25 |
Generated by irclog2html.py 4.0.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!