opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack master: Added support for specifying a maximum version of the OpenStack SDK https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839093 | 06:45 |
---|---|---|
opendevreview | Merged openstack/ansible-collections-openstack stable/1.0.0: Move dns zone info to use proxy layer https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839428 | 07:32 |
opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack stable/1.0.0: Use Rocky release of Heat in Queens job https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839327 | 08:12 |
opendevreview | Ananya proposed openstack/ansible-collections-openstack master: WIP: Moves image_info from cloud to proxy object https://review.opendev.org/c/openstack/ansible-collections-openstack/+/828158 | 08:18 |
opendevreview | Merged openstack/ansible-collections-openstack stable/1.0.0: Use Rocky release of Heat in Queens job https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839327 | 08:36 |
opendevreview | Merged openstack/ansible-collections-openstack master: Added support for specifying a maximum version of the OpenStack SDK https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839093 | 09:33 |
opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack stable/1.0.0: Added support for specifying a maximum version of the OpenStack SDK https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839349 | 09:45 |
opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack master: Removed object tags from ci role server https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839505 | 09:48 |
jm1 | gtema: hey ho :) are sdk's cloud functions going to be removed or deprecated in future? | 11:42 |
gtema | no, but it is advised to use them only when they combine multiple API calls (like upload image or create server). For other cases it is recommended to use proxy directly (nobody wants to use additional wrapping level without real need) | 11:43 |
jm1 | gtema: we are using cloud functions such as search_projects. they provide "postprocessing" filters which resource proxy functions such as identity.projects() do not. So we are unsure whether we can or should stick to cloud layer functions. we will loose the filters functionality when moving to resource proxy functions | 11:44 |
gtema | are those some specific filters? If those "can" be addressed by API filters - I would prefer to fix proxy and stick to it. If there is something special what can not be done in proxy - then you can continue using cloud layer | 11:46 |
jm1 | atm users can choose filters freely, we just pass them to the cloud layer unchanged. https://opendev.org/openstack/ansible-collections-openstack/src/branch/master/plugins/modules/identity_user_info.py#L144 | 11:49 |
jm1 | gtema: we are not sure what is the best solution to this. either continue to use cloud layer functions or change/remove filters and thus break user code? | 11:50 |
gtema | I think you can stick to cloud layer here | 11:51 |
opendevreview | Merged openstack/ansible-collections-openstack master: Removed object tags from ci role server https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839505 | 12:00 |
jm1 | gtema++ ack, thank you :) | 12:01 |
gtema | wlcm | 12:01 |
opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack master: Temporarily run passing tests in our Zuul CI jobs only https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839374 | 12:19 |
opendevreview | Arx Cruz proposed openstack/ansible-collections-openstack master: Update catalog service for the new sdk https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839352 | 12:27 |
jm1 | gtema: sdk seems to be inconsistent with postprocessing filters. some functions from cloud layer allow for applying filters to results, e.g. search_users https://opendev.org/openstack/openstacksdk/src/branch/master/openstack/cloud/_identity.py#L162 | 13:08 |
jm1 | gtema: other cloud layer functions such as search_projects do not allow filtering to the same extent because they use filters as query parameters | 13:09 |
jm1 | gtema: for example, this fails with the new sdk list(conn.search_projects(filters={"id": "123"})) | 13:10 |
jm1 | gtema: but this wont list(conn.search_users(filters={"id": "bla2"})) | 13:10 |
jm1 | gtema: in the first case (where it fails), what is the point of the postprocessing filter if you cannot use anything except for the query parameters anyway? | 13:11 |
jm1 | gtema: is this "pass all filters as query params and thus only allow valid query params as filters" a design decision which might be applied to other cloud functions such as search_users() later? | 13:13 |
jm1 | gtema: or is it the opposite, i.e. search_projects() has to be fixed to only pass filters which are valid query params to proxy functions? | 13:14 |
opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack master: Temporarily run passing tests in our Zuul CI jobs only https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839374 | 13:49 |
gtema | jm1: I would say cloud layer may need to be fixed to know what it can pass to API and filter rest. On the other side same might be applied in proxy directly. I wouldn't say there were very strict design decisions, except that proxy for now only supports what API will support (but we may rethink this) | 13:57 |
jm1 | gtema: afaik proxy layer is a slim python layer on top of the openstack rest api. hence its valid that it raises exceptions on unknown query parameters. QueryParameters and Resource classes have a allow_unknown_params flag which would allow users to force sdk to pass params to rest apis. In case of our cloud layer functions, we do not want to pass unknown params to openstack. instead we want to filter results in a postprocessing step by | 14:12 |
jm1 | parameters which are not supported as query params. | 14:12 |
jm1 | gtema: so imho cloud layer functions should only pass valid query params to proxy functions | 14:12 |
jm1 | gtema: since every resource already has a list of valid query params (_query_parameters), implementing a function to drop invalid params is kind of easy, isnt it? | 14:13 |
jm1 | gtema: then we could call this drop function from cloud layer functions such as search_projects | 14:13 |
jm1 | gtema: i am happy to help but i need some guidance on where to best add this kind of code | 14:13 |
jm1 | ..in the sdk | 14:13 |
gtema | well yes, it's not so complex, but we need then to implement it in every cloud layer function, and this is close to insanity again | 14:15 |
gtema | in proxy there is also possibility not to raise exception for unsupported query params | 14:15 |
jm1 | gtema: why insanity? we only have to add the drop function call to cloud functions which pass filters as query params | 14:16 |
gtema | so it would be possible to swap process that cloud layer passes everything possible, proxy uses only whatever supported and cloud filters again | 14:16 |
gtema | insanity because there are many cloud layer functions and we should have things working same everywhere | 14:16 |
gtema | means we need to invoke it from every list function | 14:17 |
jm1 | gtema: "in proxy there is also possibility not to raise exception for unsupported query params" => allow_unknown_params? | 14:19 |
gtema | yes | 14:19 |
gtema | but it is there primarily to allow special corner cases and need to be checked | 14:20 |
gtema | main case was that there are some APIs that do not return error if you pass unsupported query params and there are no ways to properly implement all of them (like fuzzy search), but other services might really return error if you try to pass unsupported QP | 14:21 |
gtema | so this requires some playing | 14:21 |
opendevreview | Rafael Castillo proposed openstack/ansible-collections-openstack master: Use proxy layer in identity_user_info https://review.opendev.org/c/openstack/ansible-collections-openstack/+/828539 | 14:21 |
jm1 | that is why i would not go this way. if we "simply" drop filters which are not query params, then we will never run into this issue, at the cost of openstack-api-might-return-more-than-requested | 14:24 |
gtema | sure, but then you need to implement this drop in every list function | 14:24 |
jm1 | not in any list function, only list functions which turn filters into query params like here: https://github.com/openstack/openstacksdk/blob/master/openstack/cloud/_identity.py#L54 | 14:25 |
gtema | for the sake of generalisation I would really consider implementing this deep in proxy list function (to only use what is known to work and ignore rest) | 14:25 |
jm1 | gtema: sure, do whatever fits the sdk best :) | 14:26 |
gtema | yeah, but ideally they all do this (at least that was initial idea with those filters) | 14:26 |
jm1 | gtema: ah ok. hmm.. | 14:26 |
jm1 | gtema: now i see why you prefer a generic approach | 14:27 |
gtema | that actually was the biggest point of whole R1 work - to generalize as much as possible to have same behavior across services/methods | 14:28 |
opendevreview | Rafael Castillo proposed openstack/ansible-collections-openstack master: Use proxy layer in identity_user module https://review.opendev.org/c/openstack/ansible-collections-openstack/+/828742 | 14:29 |
opendevreview | Ananya proposed openstack/ansible-collections-openstack master: WIP: Moves image_info from cloud to proxy object https://review.opendev.org/c/openstack/ansible-collections-openstack/+/828158 | 14:30 |
opendevreview | Rafael Castillo proposed openstack/ansible-collections-openstack master: Use proxy layer in identity_user module https://review.opendev.org/c/openstack/ansible-collections-openstack/+/828742 | 14:50 |
opendevreview | Rafael Castillo proposed openstack/ansible-collections-openstack master: Use proxy layer in identity_user module https://review.opendev.org/c/openstack/ansible-collections-openstack/+/828742 | 14:52 |
opendevreview | Rafael Castillo proposed openstack/ansible-collections-openstack master: Update identity_role_info for latest openstacksdk release https://review.opendev.org/c/openstack/ansible-collections-openstack/+/837751 | 15:03 |
opendevreview | Rafael Castillo proposed openstack/ansible-collections-openstack master: Use proxy layer in identity_user module https://review.opendev.org/c/openstack/ansible-collections-openstack/+/828742 | 15:09 |
opendevreview | Ananya proposed openstack/ansible-collections-openstack master: WIP: Moves image_info from cloud to proxy object https://review.opendev.org/c/openstack/ansible-collections-openstack/+/828158 | 15:12 |
jm1 | gtema: and its much better now! | 15:13 |
gtema | nice to hear | 15:14 |
jm1 | gtema: for now we will stick to cloud functions whenever we have code that passes filters. as long as users dont pass invalid query params they are fine. but it would be great if they could do the same as before ^^ | 15:15 |
gtema | ok, will think what and when to do | 15:15 |
jm1 | gtema: if you need help ping me | 15:15 |
opendevreview | Rafael Castillo proposed openstack/ansible-collections-openstack master: Update identity_role_info for latest openstacksdk release https://review.opendev.org/c/openstack/ansible-collections-openstack/+/837751 | 15:15 |
gtema | ok | 15:16 |
opendevreview | Rafael Castillo proposed openstack/ansible-collections-openstack master: Update identity_role_info for latest openstacksdk release https://review.opendev.org/c/openstack/ansible-collections-openstack/+/837751 | 15:17 |
opendevreview | Rafael Castillo proposed openstack/ansible-collections-openstack master: Update identity_role_info for latest openstacksdk release https://review.opendev.org/c/openstack/ansible-collections-openstack/+/837751 | 15:17 |
opendevreview | Rafael Castillo proposed openstack/ansible-collections-openstack master: Use proxy layer in identity_user_info https://review.opendev.org/c/openstack/ansible-collections-openstack/+/828539 | 15:20 |
opendevreview | Ananya proposed openstack/ansible-collections-openstack master: WIP: Moves image_info from cloud to proxy object https://review.opendev.org/c/openstack/ansible-collections-openstack/+/828158 | 15:22 |
opendevreview | Rafael Castillo proposed openstack/ansible-collections-openstack master: Use proxy layer in identity_user_info https://review.opendev.org/c/openstack/ansible-collections-openstack/+/828539 | 15:23 |
opendevreview | Rafael Castillo proposed openstack/ansible-collections-openstack master: Update identity_role to work with latest sdk https://review.opendev.org/c/openstack/ansible-collections-openstack/+/837772 | 15:30 |
opendevreview | Rafael Castillo proposed openstack/ansible-collections-openstack master: Update identity_domain_info to use proxy layer https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839445 | 15:40 |
opendevreview | Merged openstack/ansible-collections-openstack stable/1.0.0: Added support for specifying a maximum version of the OpenStack SDK https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839349 | 15:47 |
opendevreview | Ananya proposed openstack/ansible-collections-openstack master: WIP: Moves image_info from cloud to proxy object https://review.opendev.org/c/openstack/ansible-collections-openstack/+/828158 | 15:51 |
opendevreview | Rafael Castillo proposed openstack/ansible-collections-openstack master: Update identity_role to work with latest sdk https://review.opendev.org/c/openstack/ansible-collections-openstack/+/837772 | 16:14 |
opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack stable/1.0.0: Removed object tags from ci role server https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839545 | 18:12 |
opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack master: Constrain filters in compute_service_info to SDK >= 0.53.0 https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839546 | 18:16 |
*** artom__ is now known as artom | 18:53 | |
opendevreview | Merged openstack/ansible-collections-openstack master: Constrain filters in compute_service_info to SDK >= 0.53.0 https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839546 | 20:16 |
opendevreview | Merged openstack/ansible-collections-openstack stable/1.0.0: Removed object tags from ci role server https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839545 | 20:51 |
opendevreview | Rafael Castillo proposed openstack/ansible-collections-openstack master: Update project module to be compatible with new sdk https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839640 | 21:18 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!