*** gtema has joined #openstack-sdks | 00:35 | |
*** gtema has quit IRC | 00:40 | |
*** gtema has joined #openstack-sdks | 02:36 | |
*** gtema has quit IRC | 02:41 | |
*** factor has quit IRC | 04:31 | |
*** factor has joined #openstack-sdks | 04:31 | |
*** evrardjp has quit IRC | 05:34 | |
*** evrardjp has joined #openstack-sdks | 05:35 | |
*** gtema has joined #openstack-sdks | 06:38 | |
*** gtema has quit IRC | 06:43 | |
*** gtema has joined #openstack-sdks | 07:31 | |
*** slaweq_ has joined #openstack-sdks | 07:42 | |
*** slaweq_ is now known as slaweq | 07:45 | |
*** gtema has quit IRC | 08:01 | |
*** gtema has joined #openstack-sdks | 08:01 | |
*** jawad_axd has joined #openstack-sdks | 08:07 | |
*** gtema has quit IRC | 08:15 | |
*** tosky has joined #openstack-sdks | 08:20 | |
*** iurygregory has joined #openstack-sdks | 08:21 | |
*** gtema has joined #openstack-sdks | 08:33 | |
*** jpich has joined #openstack-sdks | 08:34 | |
*** jpena|off is now known as jpena | 08:46 | |
*** gtema has quit IRC | 08:49 | |
*** ralonsoh has joined #openstack-sdks | 08:53 | |
*** gtema has joined #openstack-sdks | 08:54 | |
*** tkajinam has quit IRC | 08:57 | |
*** gtema has quit IRC | 11:17 | |
*** sshnaidm is now known as sshnaidm|bbl | 12:09 | |
*** enriquetaso has joined #openstack-sdks | 12:19 | |
*** jpena is now known as jpena|lunch | 12:34 | |
*** gtema has joined #openstack-sdks | 12:35 | |
openstackgerrit | MichaĆ Dulko proposed openstack/openstacksdk master: Implement If-Match support for Neutron resources https://review.opendev.org/710030 | 13:12 |
---|---|---|
dulek | Folks, I could use some help with the above ^. I explained my concerns in the review. | 13:17 |
*** jpich has quit IRC | 13:20 | |
*** jpich has joined #openstack-sdks | 13:20 | |
openstackgerrit | Rodolfo Alonso Hernandez proposed openstack/python-openstackclient master: Fix network segment range "_get_ranges" function https://review.opendev.org/710031 | 13:23 |
openstackgerrit | Rodolfo Alonso Hernandez proposed openstack/python-openstackclient master: Fix network segment range "_get_ranges" function https://review.opendev.org/710031 | 13:23 |
*** jpena|lunch is now known as jpena | 13:26 | |
*** slaweq has quit IRC | 14:05 | |
*** dasp has quit IRC | 14:08 | |
*** slaweq has joined #openstack-sdks | 14:09 | |
stephenfin | hberaud, smcginnis: Can one of you hit +W on this? https://review.opendev.org/#/c/705630/ | 14:10 |
*** enriquetaso has quit IRC | 14:16 | |
*** enriquetaso has joined #openstack-sdks | 14:16 | |
openstackgerrit | Monty Taylor proposed openstack/ansible-collections-openstack master: Add a tool to build collections with pbr https://review.opendev.org/710047 | 14:41 |
gtema | mordred, would appreciate review on https://review.opendev.org/#/c/700219/ | 14:42 |
mordred | gtema: yeah - sorry - I keep opening it and then getting scared | 14:43 |
gtema | why, it's so straight forward ;-) | 14:43 |
gtema | dtantsur is now happy with it (hopefully). Since he is AFK can't verify, but we had some discussions on that | 14:44 |
mordred | gtema: yeah - it's looking good to me as I read it - nice work | 14:46 |
gtema | thks | 14:46 |
gtema | when this lands wanted to raise discussion where to make an agreed OSC plugin | 14:46 |
gtema | but anyway wanted to have a session on PTG wrt that | 14:47 |
mordred | gtema: one quibble - in project_cleanup - you've got code to handle people who are for some reason biased against threads - and that's the default code path ... why not make it use a thread pool by default and make people who want to "avoid" threads do work? (we use threads for chunked uploads to swift whether a user wants them or not, so I'm personally not even convinced we need to support avoiding | 14:47 |
mordred | them at all) | 14:47 |
gtema | dtantsur was vomplaining a lot agains that | 14:48 |
gtema | complaining | 14:48 |
mordred | one sec - gotta step away for 5 mins... | 14:48 |
gtema | what you said was a 2nd approach, but he still didn't like it | 14:48 |
gtema | ok | 14:48 |
*** iurygregory has quit IRC | 14:49 | |
mordred | k. back | 14:54 |
gtema | good | 14:54 |
mordred | gtema: so what's the issue - he's opposed to threads for some reason? | 14:54 |
mordred | oh - I see - kemme read the comments on the patch | 14:55 |
gtema | yeah, he was agains having it used in the default path | 14:55 |
*** sshnaidm|bbl is now known as sshnaidm | 14:56 | |
mordred | gtema: his original comment was just about allowing the possibilities to pass in a manager: https://review.opendev.org/#/c/700219/7/openstack/cloud/openstackcloud.py@796 | 14:56 |
gtema | I think he was explaining me more concerns in the private chat, but I lost it and forgot | 14:57 |
mordred | nod | 14:57 |
gtema | and then he said - please avoid it completely | 14:57 |
gtema | we can wait for him then for details | 14:57 |
mordred | so - I completely agree about the greenlet concerns - because people using greenlet stuff are in a general world of compexity and pain as they have no clue what their code is doing at any time :) | 14:57 |
mordred | so definitely it's important to give those folks an out | 14:58 |
gtema | :D | 14:58 |
mordred | but - I think someone writing some straightforward code should not need to opt-in to what should be normal safe behavior | 14:58 |
mordred | they shold get the best experience out of the box just by running the cleanup method | 14:59 |
gtema | I share same opinion, so we are 2 against 1. | 14:59 |
mordred | and only need to do special things if they are in special circumstances | 14:59 |
gtema | then post a review to patch and we see what he says | 14:59 |
mordred | kk | 14:59 |
mordred | btw - the greenlet concern is good - we should maybe make sure we're providing a similar escape hatch for people for other uses of executors | 15:00 |
mordred | in fact- I _think_ you can pass an executor to Connection? | 15:00 |
mordred | one sec- lemme look | 15:00 |
mordred | nope. it's a TODO | 15:01 |
mordred | gtema: so - if you look in openstack/cloud/_object_store.py | 15:01 |
mordred | there's a TODO about making self.__pool_executor configurable - for the folks in the greenlet situation | 15:01 |
mordred | maybe we should just make that a proper Connection param- then we can use self.__pool_executor in your other code | 15:02 |
mordred | and it'll also be clear to people using sdk from services how to set up a connection safely | 15:02 |
gtema | a sec, have a call | 15:02 |
mordred | (maybe this is why we were considering switching to futurist at one point?) | 15:02 |
mordred | kk. I'll leave a review | 15:02 |
gtema | back. | 15:06 |
gtema | okay, great | 15:06 |
gtema | thanks | 15:06 |
mordred | gtema: done. +2 otherwise | 15:16 |
gtema | great, thanks a lot | 15:16 |
mordred | gtema: dude - thank you for writing that | 15:16 |
gtema | welcome | 15:17 |
mordred | sorry it's taken me so long to review :) | 15:17 |
gtema | I really need it myself in lots of my projects | 15:17 |
gtema | no problems, need only to ping people some time ;-) | 15:17 |
gtema | sometime be nasty | 15:17 |
*** mugsie has quit IRC | 15:20 | |
gtema | hopefully Vancouver will not be that much affected by Corona | 15:20 |
*** mugsie has joined #openstack-sdks | 15:22 | |
mordred | gtema: we should probably review https://review.opendev.org/#/c/679914/ too | 15:28 |
gtema | oh yeah, I see I was even reviewing it already | 15:29 |
*** iurygregory has joined #openstack-sdks | 15:34 | |
*** dasp has joined #openstack-sdks | 15:52 | |
*** jawad_axd has quit IRC | 15:56 | |
dulek | mordred: Hi! Can you take a look at https://review.opendev.org/710030 ? I'm not sure how to proceed with this in an openstacksdk'ish way there. | 16:00 |
dulek | Basically the issue is that in Neutron `If-Match: revision_number=1` is the correct form, so I'd need some header modification to make this thing useful. | 16:01 |
dulek | Also usage of that header should be restricted to PUT and DELETE calls. | 16:01 |
mordred | dulek: oh my - what a fun question ... | 16:03 |
mordred | dulek: I believe we're going to get to invent a new primitive on Resource! | 16:03 |
dulek | I always engage in fun stuff. | 16:04 |
dulek | mordred: But as openstacksdk beginner I could use some advice. | 16:05 |
*** iurygregory has quit IRC | 16:05 | |
mordred | dulek: totally. I'm in the "staring off into space looking like I'm doing nothing but actually pondering your issue" state. hopefully soon I'll transition to "looking like someone who has an idea" | 16:14 |
dulek | mordred: Sure, thanks! | 16:16 |
openstackgerrit | Merged openstack/python-openstackclient stable/train: Stop silently ignoring invalid 'server create --hint' options https://review.opendev.org/705630 | 16:17 |
openstackgerrit | Merged openstack/ansible-collections-openstack master: Make an OpenStackModule base class https://review.opendev.org/698044 | 16:25 |
openstackgerrit | Merged openstack/ansible-collections-openstack master: Cleanup unit test requirements https://review.opendev.org/709113 | 16:34 |
mordred | dulek: would it be desirable do you think to have some amount of if-match happen automatically? like - if the user has a Network resource locally and goes to commit an update, should sdk automatically add an if_match="revision={current_object.revision}" if the user hasn't added one? | 16:54 |
mordred | or would that be super unexpected and unwelcome? | 16:54 |
dulek | mordred: I'd say that would be unwelcome. Neutron is not analyzing anything here, just comparing numbers and sometimes people don't care if somebody changed something in-between, they just want to rename. | 16:55 |
mordred | nod | 16:55 |
dulek | mordred: In our case we use it to make sure we won's lose an update when updating allowed_address_pairs. | 16:56 |
mordred | yah | 16:56 |
mordred | dulek: ok - so - I'm just gonna talk out loud here for a bit - this may be bong | 17:10 |
mordred | I think what you probably want to do is add an allow_if_match to openstack.resource.Resource (kind of like allow_create / allow_patch etc) ... then put some code in openstack.resource.Resource._prepare_request to add the if-match header to the headers dict if there is an if_match parameter and if allow_if_match is true ... which is then going to probably involve some annoying plumbing to allow people | 17:14 |
mordred | to pass if_match to resource.commit() and get it all the way to _prepare_request | 17:14 |
mordred | (as well as to openstack.proxy.Proxy._update) | 17:14 |
mordred | gtema: ^^ does that sound sane to you? | 17:15 |
mordred | because I agree - you don't want an if-match property on the resource - it's not actually a part of the resource | 17:15 |
gtema | lemme read quickly | 17:15 |
mordred | gtema: https://review.opendev.org/710030 has the rest of the context | 17:15 |
dulek | That sounds sane, sure, but where in prepare_request() would I have the value of user's if-match? | 17:16 |
dulek | Seems like it doesn't get any user input at the moment. | 17:16 |
dulek | Though I guess it could? | 17:16 |
dulek | Okay, I see. | 17:16 |
dulek | So the good old plumbing is the correct way. :) | 17:17 |
mordred | yeah - I think we'd want them to call either my_network_resource.commit(conn.network, if_match='revision=3') - or conn.update_network(foo='bar', if_match='revision=3') | 17:18 |
mordred | we could get fancier and make our if_match take a dict instead of a strict and construct the string for them | 17:18 |
gtema | we don't expect if-match to ever be used outside of network, right? | 17:18 |
dulek | gtema: In such form - no. | 17:18 |
dulek | mordred: I'd probably prefer conn.update_network(foo='bar', if_match_rev=3). I don't think in Neutron you're allowed to use other property names anyway. | 17:19 |
gtema | we can then do the old way with _base resource in the network service, not to have changes in real openstack.Resource | 17:20 |
mordred | hrm. if it's only ever revision and we can confirm that, yeah - i'd prefer that - or even just "if_revision" | 17:20 |
mordred | gtema: yeah. | 17:20 |
mordred | ok. yeah - seems to just be revision: https://docs.openstack.org/api-ref/network/v2/#revisions | 17:21 |
mordred | we might want to check to see if the neutron supports the revision-if-match extension too - but we can probably skip that for v1 | 17:22 |
mordred | so I'd argue for "if_revision" or something else clean like that | 17:22 |
mordred | gtema: it's more widely used | 17:23 |
gtema | okay then | 17:23 |
gtema | never noticed so far | 17:23 |
openstackgerrit | Merged openstack/openstacksdk master: Adding basic implementation for Accelerator(Cyborg) https://review.opendev.org/679914 | 17:23 |
mordred | https://docs.openstack.org/swift/pike/overview_encryption.html | 17:24 |
dulek | mordred: Yeah, but now neutron-specific changes in base resource? Because that'd be really neutron-specific, it seems. | 17:24 |
gtema | a good old swift | 17:24 |
mordred | python-cinderclient has a test that sets if-match | 17:25 |
mordred | api-sig also suggests its use, and there is an ironic spec about using it | 17:26 |
mordred | but I don't see code anywhere | 17:26 |
mordred | so - I think we could still stick with gtema's suggestion of just doing it in network since it's not *actually* widely supported with any sort of semantics that we can predict | 17:26 |
mordred | and in network we can implement it as if_revision not as generic if-match exposure | 17:27 |
mordred | which, if we grow generic if-match in the future shouldn;'t conflict | 17:27 |
gtema | yupp, sounds good for me | 17:27 |
dulek | mordred: I agree here. So base resource for neutron resources subclassing prepare_request? | 17:27 |
openstackgerrit | Artem Goncharov proposed openstack/ansible-collections-openstack master: Add volume_backup module https://review.opendev.org/710093 | 17:29 |
mordred | dulek: yeah - I think so. if you're game to take a stab at that - maybe throw up a WIP early even if it's not quite working yet and we can see how that goes | 17:30 |
mordred | it's also possible that putting it in base resource behind an "allow_if_revision" flag might hurt our brains less ... but it also might be more plumbing :) | 17:31 |
dulek | mordred: I'll see what I can do, sure. Thanks for help! | 17:31 |
gtema | ok, I "depart" for today. cu | 17:33 |
*** gtema has quit IRC | 17:34 | |
*** evrardjp has quit IRC | 17:34 | |
*** evrardjp has joined #openstack-sdks | 17:35 | |
*** jpena is now known as jpena|off | 17:40 | |
*** jpich has quit IRC | 17:48 | |
*** gtema has joined #openstack-sdks | 18:10 | |
*** gtema has quit IRC | 18:15 | |
openstackgerrit | Monty Taylor proposed openstack/ansible-collections-openstack master: Add a tool to build collections with pbr https://review.opendev.org/710047 | 18:16 |
*** tosky has quit IRC | 18:56 | |
*** sshnaidm is now known as sshnaidm|afk | 20:11 | |
*** mgariepy has quit IRC | 20:45 | |
*** ralonsoh has quit IRC | 21:22 | |
*** slaweq has quit IRC | 21:32 | |
*** enriquetaso has quit IRC | 21:57 | |
*** tosky has joined #openstack-sdks | 22:12 | |
*** sshnaidm|afk has quit IRC | 22:23 | |
*** enriquetaso has joined #openstack-sdks | 22:39 | |
*** enriquetaso has quit IRC | 22:41 | |
*** tkajinam has joined #openstack-sdks | 22:53 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!