| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-sdks - 2019-06-04 | |||
| 13:38:04 | efried | oh, I see previous alert | |
| 13:38:07 | efried | yes, cool. | |
| 13:38:18 | openstackgerrit | Monty Taylor proposed openstack/openstacksdk master: Support Proxy-specific region_name https://review.opendev.org/662865 | |
| 13:38:30 | gtema | ok, got it | |
| 13:38:50 | efried | mordred: region_name thing sane enough to fly, you think? | |
| 13:39:47 | gtema | yeah, was looking at both today morning. Looked ok for me. If mordred also thinks ok - great | |
| 13:40:00 | mordred | efried: reading that now | |
| 13:41:03 | efried | mordred: One tweak in the reno, I'll fix that up quick if you agree. | |
| 13:41:49 | mordred | efried: I'm sure I do - looking | |
| 13:42:38 | efried | I mean, yes, you're using the CloudRegion to create a Connection, not sure if a CloudRegion is good for anything else, so maybe this is a distinction without a difference. | |
| 13:42:39 | mordred | efried: yes. also - maybe it should be Connection or CloudRegion - because while you're right the primary thing is the CloudRegion- we also added a parameter to the Connection contructor | |
| 13:43:01 | mordred | who knows what people are using CloudRegions for :) | |
| 13:44:12 | efried | oh, sorry, you're right. Forest for the trees and all that. The primary use case would be Connection I guess, wouldn't it. | |
| 13:45:22 | mordred | yeah. I could go either way ... | |
| 13:45:45 | mordred | efried: what does "ddt this" mean? | |
| 13:45:46 | efried | I'll leave it alone, since we've got gtema's +2 now :) | |
| 13:46:02 | gtema | I can put another one - no problem | |
| 13:46:03 | mordred | :) | |
| 13:46:16 | efried | mordred: ddt does the same thing as testscenarios | |
| 13:46:44 | mordred | ah. is it more pleasant to use? | |
| 13:46:54 | gtema | but guys, what have you done with "/os-server-groups" with respect to microversions - all the finest things at once | |
| 13:47:17 | efried | mordred: That's probably in the eye of the beholder. Let me dig up an example quick... | |
| 13:47:32 | efried | mordred: I don't know if testscenarios works on a per-method basis; ddt does. | |
| 13:47:44 | mordred | efried: cool - I like testscenarios - but I always have to go look at previous code I wrote to figure out how to use it | |
| 13:47:54 | mordred | ah - no - testscenarios is purely class based | |
| 13:47:55 | efried | ditto ddt | |
| 13:47:56 | gtema | ++ - same for me | |
| 13:48:49 | mordred | I like the concept of putting test data into a file too | |
| 13:49:24 | efried | mordred: https://opendev.org/openstack/nova/src/branch/master/nova/tests/unit/test_nova_manage.py#L1339 | |
| 13:49:54 | efried | The data gets passed to the method as params | |
| 13:49:59 | mordred | I like that | |
| 13:50:15 | mordred | that's nicer than class attributes, which I think is a bit too magical | |
| 13:50:29 | efried | vs testscenarios where they're instance vars, but of course that difference makes sense when it's per-test | |
| 13:51:04 | efried | yeah, but it would be just as magical if they were method params, set up from the class level, wouldn't it? | |
| 13:51:21 | efried | like using a @mock.patch decorator on your class, all your methods now get a new param. | |
| 13:51:49 | mordred | oh - I don't think I've ever used mock.patch on a class :) | |
| 13:52:19 | mordred | I guess I like that in ddt the magic is localized to the method you're looking at ... but yeah, still some magic | |
| 13:52:47 | efried | I usually don't (use mock.patch on a class). If I need it to apply to all my tests, I'll useFixture(fixtures.MockPatch()) in setUp - and if I need the mock, I'll create an instance attribute :) | |
| 13:52:49 | mordred | I *really* like the file_data - I've done a few testscenarios with a BUNCH of scenarios and it quickly becomes hard to read | |
| 13:52:57 | efried | ^ manual testscenarios :) | |
| 13:53:05 | mordred | ++ to useFixture in setUp | |
| 13:54:42 | efried | ahem, "guitar" | |
| 13:54:58 | mordred | yes yes. "guitar". sorry | |
| 13:55:09 | efried | but the point of the comment was: dedup this repetitive code | |
| 13:55:21 | efried | I don't care if it's ddt or testscenarios or a helper method, whatevs. | |
| 14:00:01 | mordred | ++ | |
| 14:05:50 | openstackgerrit | Eric Fried proposed openstack/openstacksdk master: Get rid of unused _OpenStackCloudMixin.get_region https://review.opendev.org/663038 | |
| 14:20:07 | gtema | mordred: I forgot a bit about ver discovery. In the service catalog endpoint contains project_id at the end, but version info is available under compute/v2.1 | |
| 14:20:23 | gtema | the proxy is not able to discover supported microversion | |
| 14:21:11 | mordred | gtema: version discovery should be able to handle project_id at the end of endpoint | |
| 14:21:51 | gtema | well, in the small script it is not working properly | |
| 14:22:44 | gtema | wait a sec, posting | |
| 14:24:03 | gtema | mordred: http://paste.openstack.org/show/752487/ | |
| 14:24:43 | mordred | oh - you have a broken compute endpoint. it's requiring auth for version discovery | |
| 14:25:04 | mordred | see that first GET with the 401 | |
| 14:25:20 | gtema | oh, indeed | |
| 14:26:05 | mordred | I think we've got it written down somewhere that people should not auth-protect service version discovery - I don't remember where though | |
| 14:26:50 | efried | I remember this ^ but not sure if it's written | |
| 14:26:55 | gtema | hmm, ok then. Will start alerting internally on all available channels | |
| 14:27:08 | efried | It's written in IRC :) | |
| 14:27:12 | mordred | efried: :) | |
| 14:27:16 | gtema | life is great ;-/ | |
| 14:28:21 | mordred | gtema: let me know if I can be helpful in corroborating that this should not be auth-protected. | |
| 14:28:30 | mordred | s/should/must/ | |
| 14:29:40 | gtema | we probably should add it into https://opendev.org/openstack/api-sig/src/branch/master/guidelines/microversion_specification.rst | |
| 14:31:05 | mordred | gtema: yah. it's possible it's mentioned in https://review.opendev.org/#/c/459405/ - looking | |
| 14:31:18 | efried | ++. I was looking for it in the keystoneauth docs, but that's the consuming side, so probably not the right place anyway. | |
| 14:32:33 | mordred | https://review.opendev.org/#/c/459405/ | |
| 14:32:52 | mordred | https://review.opendev.org/#/c/459710/17/guidelines/discoverability.rst@93 | |
| 14:33:38 | gtema | hmm, 'it is recommended' is confusing. It is definitely not working at all otherwise | |
| 14:35:18 | efried | it's possible to make it work, I suppose. "strongly recommended"? ;P | |
| 14:35:32 | gtema | :D | |
| 14:36:02 | gtema | as a consumer I doubt you will make it work. If we bake it in - sure | |
| 14:37:08 | mordred | yeah - we should probably change that to be stronger | |
| 14:38:01 | mordred | it's unpossible to make it work with the official openstack libraries or any of the tools. obviously you could make it work with direct REST calls ... | |
| 14:38:12 | gtema | I guess it is either stricktly required, or possible otherwise (not as of now) | |
| 14:38:24 | gtema | so yeah - exactly what I mean | |
| 14:38:27 | mordred | ++ | |
| 14:39:03 | mordred | it's also not a thing we can update keystoneauth to handle reasonably - it would take basically a full rewrite IIRC | |
| 14:39:50 | gtema | ok. Sad, means cloud is still cloudy, but that is how it is | |
| 14:40:10 | mordred | :( - sorry about that | |
| 14:41:01 | gtema | another question for the same topic. Some stuff require 'min' version. Do we have something to pick the proper version out of it (not sticking for this min if current is better)? | |
| 14:41:03 | mordred | we should really add a tempest test and get refstack to require that it works properly | |
| 14:41:16 | gtema | that is a cool idea | |
| 14:41:46 | efried | ++ | |
| 14:43:09 | mordred | gtema: dtantsur added some stuff to the resource layer for specifying microversions needed for things - | |
| 14:43:12 | mordred | there's also https://opendev.org/openstack/openstacksdk/src/branch/master/openstack/compute/v2/server.py#L421 | |
| 14:43:18 | mordred | how we're doing it for live migrations | |
| 14:44:01 | mordred | there should probably be some sort of helper pattern extracted from that, because that's super manual | |
| 14:44:06 | gtema | the question is basically coming from: /servers/.../createImage returns image_id only after 2.45. And do not want to simply say - use 2.45 here. I would like to treat microver as a "num" | |
| 14:44:27 | gtema | to get the newest | |
| 14:46:02 | mordred | I think what the current resource microversion stuff is saying is "This resource knows how to handle microversions as new as 2.45" - because just doing latest could mean semantic changes that the resource can't handle | |
| 14:47:15 | mordred | then, in the code, if we wanted to make image_id part of our API all the time, we might need to make an extra call if the cloud is older than 2.45, yeah? | |
| 14:47:30 | gtema | yeah, for server-group it is slightly more complicated - until 2.63 there is attr 'policies' | |
| 14:47:57 | gtema | after 2.64 it is 'policy' and a single only | |
| 14:48:05 | mordred | *awesome* | |
| 14:48:43 | mordred | can there be more than one policy in the list until 2.63? or is it a list that happened to only ever have one policy in it? | |
| 14:48:50 | gtema | so I need to somehow check - do we support max something older - than do one cool thing. If we support something newer than 2.64 - do other cool thing | |
| 14:49:03 | gtema | nope, should not be | |
| 14:49:14 | gtema | stated in docs - list with strictly one entity | |