Wednesday, 2020-02-26

*** gtema has joined #openstack-sdks00:35
*** gtema has quit IRC00:40
*** gtema has joined #openstack-sdks02:36
*** gtema has quit IRC02:41
*** factor has quit IRC04:31
*** factor has joined #openstack-sdks04:31
*** evrardjp has quit IRC05:34
*** evrardjp has joined #openstack-sdks05:35
*** gtema has joined #openstack-sdks06:38
*** gtema has quit IRC06:43
*** gtema has joined #openstack-sdks07:31
*** slaweq_ has joined #openstack-sdks07:42
*** slaweq_ is now known as slaweq07:45
*** gtema has quit IRC08:01
*** gtema has joined #openstack-sdks08:01
*** jawad_axd has joined #openstack-sdks08:07
*** gtema has quit IRC08:15
*** tosky has joined #openstack-sdks08:20
*** iurygregory has joined #openstack-sdks08:21
*** gtema has joined #openstack-sdks08:33
*** jpich has joined #openstack-sdks08:34
*** jpena|off is now known as jpena08:46
*** gtema has quit IRC08:49
*** ralonsoh has joined #openstack-sdks08:53
*** gtema has joined #openstack-sdks08:54
*** tkajinam has quit IRC08:57
*** gtema has quit IRC11:17
*** sshnaidm is now known as sshnaidm|bbl12:09
*** enriquetaso has joined #openstack-sdks12:19
*** jpena is now known as jpena|lunch12:34
*** gtema has joined #openstack-sdks12:35
openstackgerritMichaƂ Dulko proposed openstack/openstacksdk master: Implement If-Match support for Neutron resources  https://review.opendev.org/71003013:12
dulekFolks, I could use some help with the above ^. I explained my concerns in the review.13:17
*** jpich has quit IRC13:20
*** jpich has joined #openstack-sdks13:20
openstackgerritRodolfo Alonso Hernandez proposed openstack/python-openstackclient master: Fix network segment range "_get_ranges" function  https://review.opendev.org/71003113:23
openstackgerritRodolfo Alonso Hernandez proposed openstack/python-openstackclient master: Fix network segment range "_get_ranges" function  https://review.opendev.org/71003113:23
*** jpena|lunch is now known as jpena13:26
*** slaweq has quit IRC14:05
*** dasp has quit IRC14:08
*** slaweq has joined #openstack-sdks14:09
stephenfinhberaud, smcginnis: Can one of you hit +W on this? https://review.opendev.org/#/c/705630/14:10
*** enriquetaso has quit IRC14:16
*** enriquetaso has joined #openstack-sdks14:16
openstackgerritMonty Taylor proposed openstack/ansible-collections-openstack master: Add a tool to build collections with pbr  https://review.opendev.org/71004714:41
gtemamordred, would appreciate review on https://review.opendev.org/#/c/700219/14:42
mordredgtema: yeah - sorry - I keep opening it and then getting scared14:43
gtemawhy, it's so straight forward ;-)14:43
gtemadtantsur is now happy with it (hopefully). Since he is AFK can't verify, but we had some discussions on that14:44
mordredgtema: yeah - it's looking good to me as I read it - nice work14:46
gtemathks14:46
gtemawhen this lands wanted to raise discussion where to make an agreed OSC plugin14:46
gtemabut anyway wanted to have a session on PTG wrt that14:47
mordredgtema: 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 avoiding14:47
mordredthem at all)14:47
gtemadtantsur was vomplaining a lot agains that14:48
gtemacomplaining14:48
mordredone sec - gotta step away for 5 mins...14:48
gtemawhat you said was a 2nd approach, but he still didn't like it14:48
gtemaok14:48
*** iurygregory has quit IRC14:49
mordredk. back14:54
gtemagood14:54
mordredgtema: so what's the issue - he's opposed to threads for some reason?14:54
mordredoh - I see - kemme read the comments on the patch14:55
gtemayeah, he was agains having it used in the default path14:55
*** sshnaidm|bbl is now known as sshnaidm14:56
mordredgtema: 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@79614:56
gtemaI think he was explaining me more concerns in the private chat, but I lost it and forgot14:57
mordrednod14:57
gtemaand then he said - please avoid it completely14:57
gtemawe can wait for him then for details14:57
mordredso - 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
mordredso definitely it's important to give those folks an out14:58
gtema:D14:58
mordredbut - I think someone writing some straightforward code should not need to opt-in to what should be normal safe behavior14:58
mordredthey shold get the best experience out of the box just by running the cleanup method14:59
gtemaI share same opinion, so we are 2 against 1.14:59
mordredand only need to do special things if they are in special circumstances14:59
gtemathen post a review to patch and we see what he says14:59
mordredkk14:59
mordredbtw - the greenlet concern is good - we should maybe make sure we're providing a similar escape hatch for people for other uses of executors15:00
mordredin fact- I _think_ you can pass an executor to Connection?15:00
mordredone sec- lemme look15:00
mordrednope. it's a TODO15:01
mordredgtema: so - if you look in openstack/cloud/_object_store.py15:01
mordredthere's a TODO about making self.__pool_executor configurable - for the folks in the greenlet situation15:01
mordredmaybe we should just make that a proper Connection param- then we can use self.__pool_executor in your other code15:02
mordredand it'll also be clear to people using sdk from services how to set up a connection safely15:02
gtemaa sec, have a call15:02
mordred(maybe this is why we were considering switching to futurist at one point?)15:02
mordredkk. I'll leave a review15:02
gtemaback.15:06
gtemaokay, great15:06
gtemathanks15:06
mordredgtema: done. +2 otherwise15:16
gtemagreat, thanks a lot15:16
mordredgtema: dude - thank you for writing that15:16
gtemawelcome15:17
mordredsorry it's taken me so long to review :)15:17
gtemaI really need it myself in lots of my projects15:17
gtemano problems, need only to ping people some time ;-)15:17
gtemasometime be nasty15:17
*** mugsie has quit IRC15:20
gtemahopefully Vancouver will not be that much affected by Corona15:20
*** mugsie has joined #openstack-sdks15:22
mordredgtema: we should probably review https://review.opendev.org/#/c/679914/ too15:28
gtemaoh yeah, I see I was even reviewing it already15:29
*** iurygregory has joined #openstack-sdks15:34
*** dasp has joined #openstack-sdks15:52
*** jawad_axd has quit IRC15:56
dulekmordred: 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
dulekBasically 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
dulekAlso usage of that header should be restricted to PUT and DELETE calls.16:01
mordreddulek: oh my - what a fun question ...16:03
mordreddulek: I believe we're going to get to invent a new primitive on Resource!16:03
dulekI always engage in fun stuff.16:04
dulekmordred: But as openstacksdk beginner I could use some advice.16:05
*** iurygregory has quit IRC16:05
mordreddulek: 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
dulekmordred: Sure, thanks!16:16
openstackgerritMerged openstack/python-openstackclient stable/train: Stop silently ignoring invalid 'server create --hint' options  https://review.opendev.org/70563016:17
openstackgerritMerged openstack/ansible-collections-openstack master: Make an OpenStackModule base class  https://review.opendev.org/69804416:25
openstackgerritMerged openstack/ansible-collections-openstack master: Cleanup unit test requirements  https://review.opendev.org/70911316:34
mordreddulek: 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
mordredor would that be super unexpected and unwelcome?16:54
dulekmordred: 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
mordrednod16:55
dulekmordred: In our case we use it to make sure we won's lose an update when updating allowed_address_pairs.16:56
mordredyah16:56
mordreddulek: ok - so - I'm just gonna talk out loud here for a bit - this may be bong17:10
mordredI 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 people17:14
mordredto pass if_match to resource.commit() and get it all the way to _prepare_request17:14
mordred(as well as to openstack.proxy.Proxy._update)17:14
mordredgtema: ^^ does that sound sane to you?17:15
mordredbecause I agree - you don't want an if-match property on the resource - it's not actually a part of the resource17:15
gtemalemme read quickly17:15
mordredgtema: https://review.opendev.org/710030 has the rest of the context17:15
dulekThat sounds sane, sure, but where in prepare_request() would I have the value of user's if-match?17:16
dulekSeems like it doesn't get any user input at the moment.17:16
dulekThough I guess it could?17:16
dulekOkay, I see.17:16
dulekSo the good old plumbing is the correct way. :)17:17
mordredyeah - 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
mordredwe could get fancier and make our if_match take a dict instead of a strict and construct the string for them17:18
gtemawe don't expect if-match to ever be used outside of network, right?17:18
dulekgtema: In such form - no.17:18
dulekmordred: 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
gtemawe can then do the old way with _base resource in the network service, not to have changes in real openstack.Resource17:20
mordredhrm. if it's only ever revision and we can confirm that, yeah - i'd prefer that - or even just "if_revision"17:20
mordredgtema: yeah.17:20
mordredok. yeah - seems to just be revision: https://docs.openstack.org/api-ref/network/v2/#revisions17:21
mordredwe might want to check to see if the neutron supports the revision-if-match extension too - but we can probably skip that for v117:22
mordredso I'd argue for "if_revision" or something else clean like that17:22
mordredgtema: it's more widely used17:23
gtemaokay then17:23
gtemanever noticed so far17:23
openstackgerritMerged openstack/openstacksdk master: Adding basic implementation for Accelerator(Cyborg)  https://review.opendev.org/67991417:23
mordredhttps://docs.openstack.org/swift/pike/overview_encryption.html17:24
dulekmordred: Yeah, but now neutron-specific changes in base resource? Because that'd be really neutron-specific, it seems.17:24
gtemaa good old swift17:24
mordredpython-cinderclient has a test that sets if-match17:25
mordredapi-sig also suggests its use, and there is an ironic spec about using it17:26
mordredbut I don't see code anywhere17:26
mordredso - 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 predict17:26
mordredand in network we can implement it as if_revision not as generic if-match exposure17:27
mordredwhich, if we grow generic if-match in the future shouldn;'t conflict17:27
gtemayupp, sounds good for me17:27
dulekmordred: I agree here. So base resource for neutron resources subclassing prepare_request?17:27
openstackgerritArtem Goncharov proposed openstack/ansible-collections-openstack master: Add volume_backup module  https://review.opendev.org/71009317:29
mordreddulek: 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 goes17:30
mordredit'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
dulekmordred: I'll see what I can do, sure. Thanks for help!17:31
gtemaok, I "depart" for today. cu17:33
*** gtema has quit IRC17:34
*** evrardjp has quit IRC17:34
*** evrardjp has joined #openstack-sdks17:35
*** jpena is now known as jpena|off17:40
*** jpich has quit IRC17:48
*** gtema has joined #openstack-sdks18:10
*** gtema has quit IRC18:15
openstackgerritMonty Taylor proposed openstack/ansible-collections-openstack master: Add a tool to build collections with pbr  https://review.opendev.org/71004718:16
*** tosky has quit IRC18:56
*** sshnaidm is now known as sshnaidm|afk20:11
*** mgariepy has quit IRC20:45
*** ralonsoh has quit IRC21:22
*** slaweq has quit IRC21:32
*** enriquetaso has quit IRC21:57
*** tosky has joined #openstack-sdks22:12
*** sshnaidm|afk has quit IRC22:23
*** enriquetaso has joined #openstack-sdks22:39
*** enriquetaso has quit IRC22:41
*** tkajinam has joined #openstack-sdks22:53

Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!