Earlier  
Later
New Search
Posted Nick Remark
#openstack-sdks - 2020-03-02
12:53:59 larsks How does one pass all_tenants=true to the list_volumes method in python-openstacksdk?
13:27:50 openstackgerrit Merged openstack/ansible-collections-openstack master: Remove old artifacts when building new ones https://review.opendev.org/710293
13:31:33 openstackgerrit Merged openstack/ansible-collections-openstack master: Run flake8 in linters https://review.opendev.org/710294
13:31:34 openstackgerrit Merged openstack/ansible-collections-openstack master: Fix F841 and remove exclusion https://review.opendev.org/698063
13:31:35 openstackgerrit Merged openstack/ansible-collections-openstack master: Remove F403 and F405 exclusions https://review.opendev.org/710295
13:31:36 openstackgerrit Merged openstack/ansible-collections-openstack master: Fix E128 and remove exclusion https://review.opendev.org/698064
13:31:37 openstackgerrit Merged openstack/ansible-collections-openstack master: Fix W504 and remove exclusion https://review.opendev.org/710296
13:31:38 openstackgerrit Merged openstack/ansible-collections-openstack master: Fix F401 and remove exclusion https://review.opendev.org/698065
15:38:12 dtantsur mordred: http://lists.openstack.org/pipermail/openstack-discuss/2020-March/012928.html Oo
15:38:38 mordred dtantsur: uhm
15:39:05 brtknr mordred: since when did openstacksdk start to replace uuid keys with id?
15:39:10 brtknr dtantsur: ^
15:39:53 brtknr for get requests
15:43:39 mordred brtknr: since always. the REST interactions should use uuid as the key name if it needs to, and uuid _should_ be available as an alias - but it's a primary design of SDK that every object returned has an "id" field that is the id of the object
15:44:44 mordred dtantsur: I have also responded to the thread, possibly a bit tersely
15:45:01 dtantsur cool, that's what I expected :)
15:59:12 mordred dtantsur: if you get bored, I left a disagreement on https://review.opendev.org/#/c/700219/ - I think we should probably come to an agreement amongst ourselves
16:09:06 dtantsur mordred: I like the idea of a global executor and more or less okay with defaulting to threads as long as it can be completely overridden
16:10:33 tosky just to bring the discussion in the right place, one of the blocker raised by the cinder people was and is https://review.opendev.org/#/c/590807/
16:10:38 tosky and the lack of updates on it
16:10:44 tosky is there any update on it?
16:11:54 mordred dtantsur: cool!
16:12:03 mordred tosky: looking
16:15:45 mordred tosky: 3 things. a) I'd still prefer migrating to sdk there, since microversion support is solid and we use it in other places and all. HOWEVER b) I don't think we should block forward progress on that, we can certainly migrate later c) that patch should probably be fixed to pass tests - all of the previous blockers that existed at the time of that patch originally should not exist anymore
16:18:25 tosky mordred: that's good to hear (all of it) - could you please comment on the patch? :)
16:19:18 brtknr mordred: so uuid not being available too is possibly a bug?
16:20:33 mordred brtknr: yeah, possibly so. where are you seeing the issue?
16:20:56 brtknr for os_coe_cluster and os_coe_cluster_template modules, only id is returned, not uuid
16:21:24 brtknr looks like the corresponding ansible module expects to see uuid field which leads to issues with idempotency
16:21:41 mordred brtknr: that's super weird :)
16:21:46 brtknr sorry the get_coe_cluster and get_coe_cluster_template functions
16:22:42 mordred brtknr: oh - that's even weirder. that's teh cloud layer, becuase it's for ansible. I'm not sure that's _ever_ returned uuid - so I have no idea how those ansible modules worked? looking further though .,..
16:23:04 brtknr mordred: wait, only get_coe_cluster has this problem
16:23:15 brtknr get_coe_cluster_template seems to return uuid as an alias
16:24:09 mordred brtknr: yes - I agree with you
16:24:18 mordred cluster template adds it ba k
16:24:37 brtknr mordred: can you show me the code?
16:25:04 brtknr mordred: dont worry, i will try to find it myself, i should stop being lazy
16:25:09 mordred brtknr: https://opendev.org/openstack/openstacksdk/src/branch/master/openstack/cloud/_normalize.py#L1026-L1032 is where cluster_template adds it back if strict == False
16:25:30 mordred and you can see that https://opendev.org/openstack/openstacksdk/src/branch/master/openstack/cloud/_normalize.py#L971-L997 doens't do a similar thing
16:26:07 brtknr ouch
16:26:29 brtknr who'd like to propose a patch?
16:26:53 brtknr I'm happy to if people are busy :)
16:28:07 brtknr Actually I am not sure what this strict_mode is about
16:31:14 brtknr found the reno: https://github.com/openstack/openstacksdk/blob/master/releasenotes/notes/strict-mode-d493abc0c3e87945.yaml
16:35:53 mordred brtknr: the idea was that we'd have a consistent interface peopel could count on - as well as fallback values
16:36:28 mordred however - that concept pre-dates the merge of shade and sdk. with the sdk objects we have the opportunity to define aliases - and that seems to be a better long-term experience for users anyway
16:37:03 mordred we've been working on shifting the shade layer to using the sdk resource objects and getting rid of that normalize layer - but havne't made it to the coe methods yet
16:37:16 openstackgerrit Rodolfo Alonso Hernandez proposed openstack/openstacksdk master: Include "fields" to "SecurityGroup" query parameters https://review.opendev.org/710820
16:37:58 mordred brtknr: if you have a sec, you could probably just cargo-cult in an if not strict_mode: add back uuid patch to keep it simple for the time being - and we can see about migrating the methods as a followup
16:39:05 larsks mordred: Do you know if there is a particular reason that volume.volumes(details=True) doesn't return the user_id information from the remote response?
16:41:04 openstackgerrit Bharat Kunwar proposed openstack/openstacksdk master: Return uuid alias for coe_cluster in non strict mode https://review.opendev.org/710823
16:41:08 brtknr mordred: ^
16:41:36 mordred larsks: looking
16:42:09 mordred brtknr: looks great. it's possible a test mock will need to get updated as a result - but let's see how it goes
16:42:31 openstackgerrit Rodolfo Alonso Hernandez proposed openstack/python-openstackclient master: Add "fields" parameter to ListSecurityGroup query https://review.opendev.org/710825
16:43:34 mordred larsks: there is no mention of user_id in openstack/block_storage/v3/volume.py - so I think it's purely that it's missing and not anything on purpose
16:43:56 larsks mordred: thanks. I was just curious if the omission was intentional or not. I'll submit a change.
16:45:08 brtknr mordred: locally, all my tox -e py36 failures are related to cloud devstack not found as I export OS_CLOUD=devstack to my env vars
16:46:02 mordred brtknr: hah. nice
16:46:12 mordred brtknr: well - let's see what zuul tells us :)
16:46:23 brtknr mordred: also TIL what cargo cult means
16:46:42 mordred brtknr: :)
16:47:19 brtknr If this patch goes through, it will void this patch: https://review.opendev.org/#/c/709734/
16:48:44 brtknr py36: commands succeeded
16:48:46 brtknr congratulations :)
16:48:57 brtknr after unsetting OS_CLOUD
16:49:26 mordred brtknr: I think we should land that patch too - it describes the current state - and somone might have those modules and _not_ latest sdk - and the id codepath you did is always going to work
16:50:00 mordred when the uuid codepath lands in sdk it'll improve things for people and then we can add a doc that says uuid will be an alias in the return value if sdk is new enough
16:50:07 brtknr hmm this seems to highlight an underlying problem, why is tox -e py36 fail when I export OS_CLOUD=
16:50:28 brtknr mordred: sounds good to me
16:50:37 brtknr I will revert the change to the doc
16:50:54 brtknr btw how the hell do you use ansible-collection-openstack?
16:51:16 brtknr Its not available on galaxy yet so cant use ansible-galaxy to install it
16:51:42 smcginnis brtknr: I haven't been following the conversation here, but sound like the OS_CLOUD issue might be a side effect of this: https://opendev.org/openstack/ansible-collections-openstack/src/branch/master/tox.ini#L10
16:51:56 mordred brtknr: no - don't revert it - I think it's solid
16:52:01 mordred brtknr: it's the right thing
16:52:21 mordred brtknr: you need to build a tarball from source then you can install the tarball
16:52:30 mordred we're working on getting galaxy pub working
16:52:43 mordred brtknr: I recommend using tox -ebuild to make the tarball
16:52:50 mordred otherwise it's going to have a sad
16:52:59 brtknr revert line 190 here: https://review.opendev.org/#/c/709734/4/plugins/modules/os_coe_cluster.py
16:53:16 mordred no - don't revert that
16:53:21 mordred your comment there is correct
16:53:43 brtknr Ah okay fair enough
16:53:51 mordred that _is_ the situation most users will encounter - they won't see uuid unless they install an as-yet-unreleased sdk :)
16:54:17 brtknr mordred: is it correct that when a cluster gets *created* then uuid is returned?
16:54:35 brtknr shouldn't the uuid get normalised in the same way for create requests too?
16:54:38 mordred it shouldn't - it should be the same data structure all the time
16:54:40 mordred yup
16:55:07 brtknr ok so thats another bummer, it returns uuid at present
16:55:15 brtknr when cluster get created
16:55:29 mordred wow really?
16:56:09 brtknr yes sorry
16:56:20 mordred so that means we're not running normalize on the result of create - which is sadmaking. maybe we need to do a workaround in the ansible module itself
16:56:53 mordred to grab the id in both cases with a get('id', get('uuid') - then make sure both id and uuid are set in the dict before we return to teh ansible user
16:57:03 mordred cause otherwise this is just a mess for the user
16:58:31 brtknr that is the premise of the patch
16:58:50 brtknr its what i am already proposing we do with that patch
17:00:01 mordred cool. then yes - I think we should land that patch asap

Earlier   Later