Earlier  
Later
New Search
Posted Nick Remark
#openstack-sdks - 2020-02-26
15:06:09 gtema okay, great
15:06:12 gtema thanks
15:16:43 mordred gtema: done. +2 otherwise
15:16:52 gtema great, thanks a lot
15:16:59 mordred gtema: dude - thank you for writing that
15:17:09 gtema welcome
15:17:10 mordred sorry it's taken me so long to review :)
15:17:20 gtema I really need it myself in lots of my projects
15:17:33 gtema no problems, need only to ping people some time ;-)
15:17:39 gtema sometime be nasty
15:20:54 gtema hopefully Vancouver will not be that much affected by Corona
15:28:32 mordred gtema: we should probably review https://review.opendev.org/#/c/679914/ too
15:29:09 gtema oh yeah, I see I was even reviewing it already
16:00:49 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:01:37 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:57 dulek Also usage of that header should be restricted to PUT and DELETE calls.
16:03:13 mordred dulek: oh my - what a fun question ...
16:03:56 mordred dulek: I believe we're going to get to invent a new primitive on Resource!
16:04:54 dulek I always engage in fun stuff.
16:05:24 dulek mordred: But as openstacksdk beginner I could use some advice.
16:14:17 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:16:20 dulek mordred: Sure, thanks!
16:17:24 openstackgerrit Merged openstack/python-openstackclient stable/train: Stop silently ignoring invalid 'server create --hint' options https://review.opendev.org/705630
16:25:33 openstackgerrit Merged openstack/ansible-collections-openstack master: Make an OpenStackModule base class https://review.opendev.org/698044
16:34:50 openstackgerrit Merged openstack/ansible-collections-openstack master: Cleanup unit test requirements https://review.opendev.org/709113
16:54:20 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:41 mordred or would that be super unexpected and unwelcome?
16:55:51 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:58 mordred nod
16:56:20 dulek mordred: In our case we use it to make sure we won's lose an update when updating allowed_address_pairs.
16:56:30 mordred yah
17:10:39 mordred dulek: ok - so - I'm just gonna talk out loud here for a bit - this may be bong
17:14:18 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:20 mordred to pass if_match to resource.commit() and get it all the way to _prepare_request
17:14:59 mordred (as well as to openstack.proxy.Proxy._update)
17:15:12 mordred gtema: ^^ does that sound sane to you?
17:15:32 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:33 gtema lemme read quickly
17:15:57 mordred gtema: https://review.opendev.org/710030 has the rest of the context
17:16:19 dulek That sounds sane, sure, but where in prepare_request() would I have the value of user's if-match?
17:16:28 dulek Seems like it doesn't get any user input at the moment.
17:16:33 dulek Though I guess it could?
17:16:56 dulek Okay, I see.
17:17:19 dulek So the good old plumbing is the correct way. :)
17:18:03 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:24 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:39 gtema we don't expect if-match to ever be used outside of network, right?
17:18:47 dulek gtema: In such form - no.
17:19:21 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:20:25 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:29 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:44 mordred gtema: yeah.
17:21:44 mordred ok. yeah - seems to just be revision: https://docs.openstack.org/api-ref/network/v2/#revisions
17:22:16 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:30 mordred so I'd argue for "if_revision" or something else clean like that
17:23:14 mordred gtema: it's more widely used
17:23:32 gtema okay then
17:23:39 gtema never noticed so far
17:23:46 openstackgerrit Merged openstack/openstacksdk master: Adding basic implementation for Accelerator(Cyborg) https://review.opendev.org/679914
17:24:08 mordred https://docs.openstack.org/swift/pike/overview_encryption.html
17:24:15 dulek mordred: Yeah, but now neutron-specific changes in base resource? Because that'd be really neutron-specific, it seems.
17:24:32 gtema a good old swift
17:25:29 mordred python-cinderclient has a test that sets if-match
17:26:05 mordred api-sig also suggests its use, and there is an ironic spec about using it
17:26:08 mordred but I don't see code anywhere
17:26:48 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:27:06 mordred and in network we can implement it as if_revision not as generic if-match exposure
17:27:17 mordred which, if we grow generic if-match in the future shouldn;'t conflict
17:27:34 gtema yupp, sounds good for me
17:27:44 dulek mordred: I agree here. So base resource for neutron resources subclassing prepare_request?
17:29:07 openstackgerrit Artem Goncharov proposed openstack/ansible-collections-openstack master: Add volume_backup module https://review.opendev.org/710093
17:30:18 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:31:04 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:32 dulek mordred: I'll see what I can do, sure. Thanks for help!
17:33:37 gtema ok, I "depart" for today. cu
18:16:01 openstackgerrit Monty Taylor proposed openstack/ansible-collections-openstack master: Add a tool to build collections with pbr https://review.opendev.org/710047
#openstack-sdks - 2020-02-27
13:32:54 openstackgerrit Rodolfo Alonso Hernandez proposed openstack/python-openstackclient master: Fix network segment range "_get_ranges" function https://review.opendev.org/710031
14:20:47 openstackgerrit Monty Taylor proposed openstack/ansible-collections-openstack master: Add a tool to build collections with pbr https://review.opendev.org/710047
15:29:29 openstackgerrit Monty Taylor proposed openstack/ansible-collections-openstack master: Fix F841 and remove exclusion https://review.opendev.org/698063
15:29:30 openstackgerrit Monty Taylor proposed openstack/ansible-collections-openstack master: Fix E128 and remove exclusion https://review.opendev.org/698064
15:29:31 openstackgerrit Monty Taylor proposed openstack/ansible-collections-openstack master: Fix F401 and remove exclusion https://review.opendev.org/698065
15:29:32 openstackgerrit Monty Taylor proposed openstack/ansible-collections-openstack master: Remove old artifacts when building new ones https://review.opendev.org/710293
15:29:33 openstackgerrit Monty Taylor proposed openstack/ansible-collections-openstack master: Run flake8 in linters https://review.opendev.org/710294
15:29:34 openstackgerrit Monty Taylor proposed openstack/ansible-collections-openstack master: Remove F403 and F405 exclusions https://review.opendev.org/710295
15:29:35 openstackgerrit Monty Taylor proposed openstack/ansible-collections-openstack master: Fix W504 and remove exclusion https://review.opendev.org/710296
15:52:49 openstackgerrit Michał Dulko proposed openstack/openstacksdk master: Implement If-Match support for Neutron resources https://review.opendev.org/710030
15:56:04 openstackgerrit Michał Dulko proposed openstack/openstacksdk master: Implement If-Match support for Neutron resources https://review.opendev.org/710030
16:00:18 elmiko API SIG office hour now open
16:00:25 openstackgerrit Michał Dulko proposed openstack/openstacksdk master: Implement If-Match support for Neutron resources https://review.opendev.org/710030
16:02:45 gtema hi elmiko. So with PTG it's all clear, right?
16:03:23 dulek mordred, gtema: Alright, so here's my attempt to implement that thing with the simple plumbing: https://review.opendev.org/#/c/710030.
16:03:27 dulek Tell me what you think!
16:05:28 gtema dulek, will have a look, but unfort. not today - a total madness today
16:05:53 dulek gtema: Well, this thing isn't really urgent.
16:07:55 gtema that's good
16:09:16 elmiko gtema: is it?
16:09:55 gtema elmiko - it is, nobody cares. We do what we want
16:14:05 mordred dulek: that's looking good!
16:15:06 mordred elmiko, gtema: I requested a day from kendall for SDK/OSC/ansible-openstack - I think that could also easily include API ... basically a day we can divy up however
16:15:52 gtema yupp, thanks mordred. With additional 1/4 day dtantsur requested specifically for API we are absolutely ok

Earlier   Later