| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-sdks - 2017-02-16 | |||
| 15:40:48 | dtroyer | cliff hasn't changed, I did wonder about the wisdom of pulling in tempest there, but figured utilities were safe | |
| 15:46:05 | stevemar | dtroyer: looking at the source it seems okay... | |
| 15:48:21 | stevemar | dtroyer: tempest looks OK, i'm quite confused :) | |
| 15:49:09 | stevemar | dtroyer: but i definitely don't see something that we merged that would have broken us | |
| 15:49:12 | dtroyer | its the unicode coercion that struck me… does tempest include unicode chars in the generated name, maybe only occasionally? | |
| 15:50:34 | stevemar | dtroyer: thats what i was wondering, why i put up the patch to use a straight uuid | |
| 15:50:52 | stevemar | dtroyer: but it doesn't seem like tempest does that https://github.com/openstack/tempest/blob/master/tempest/lib/common/utils/data_utils.py#L46-L62 | |
| 15:50:57 | dtroyer | which I think we should do anyway | |
| 15:51:56 | dtroyer | yeah, that's pretty simple | |
| 16:17:33 | mordred | cdent: what channel is the api meeting in again? | |
| 16:17:48 | cdent | mordred: #openstack-meeting-3 | |
| 16:34:18 | openstackgerrit | Michael Johnson proposed openstack/service-types-authority master: Add load-balancing service type https://review.openstack.org/434999 | |
| 16:38:06 | openstackgerrit | Merged openstack/api-wg master: Add guidelines for boolean names https://review.openstack.org/411529 | |
| 19:01:15 | openstackgerrit | Rodolfo Alonso Hernandez proposed openstack/python-openstackclient master: Add new parameter "is_default" to Network QoS policy. https://review.openstack.org/432260 | |
| 19:11:55 | openstackgerrit | Brian Curtin proposed openstack/python-openstacksdk master: Reorganize key_manager docs https://review.openstack.org/435069 | |
| 19:16:14 | ankur-gupta-f4 | dtroyer: finalize --<agent-name> for the network agent commands. Put the optional arg. Done-done. | |
| 19:16:43 | dtroyer | ok, cool, I'll chekc when I pass through the review queue again later | |
| 19:17:27 | sindhu | dtroyer: ping? | |
| 19:17:49 | dtroyer | sindhu: yes | |
| 19:18:08 | sindhu | dtroyer: hi, regarding https://review.openstack.org/#/c/382023/ | |
| 19:18:55 | sindhu | dtroyer: is it ok to have --network, --compute, --volume option like the list command? | |
| 19:19:17 | sindhu | dtroyer: right now this patch only handles network extension | |
| 19:19:53 | dtroyer | what other API still has extensions? They've been eradicated from the other core projects | |
| 19:19:59 | ankur-gupta-f4 | the others don't have extension show though do they? | |
| 19:20:01 | ankur-gupta-f4 | yea | |
| 19:20:10 | dtroyer | and I'd argue should go away in neutron too, but that's for a different audience | |
| 19:20:51 | ankur-gupta-f4 | hence my comment to take it out of common and make it a core networking command. so 'os network extension show' | |
| 19:20:54 | dtroyer | that said, I dislike making the resource name 'network extension' because it reinforces the notion that "all network commands must be namespaced with 'network'" which is exactly not the case | |
| 19:21:14 | reedip_1 | dtroyer : Neutron is pretty tightly coupled with extensions actually | |
| 19:21:45 | dtroyer | OSC is based around named resources, many of which are qualified with names that happen to match API service types, but some do not. and | |
| 19:21:55 | reedip_1 | I agree with ankur-gupta-f4 , remove this from common till we do not have more extensions to list :P | |
| 19:22:09 | dtroyer | reedip_: that doesn't make it a good idea or one that should be copied elsewhere. it isn't | |
| 19:23:38 | reedip_1 | dtroyer : I agree , if it has been removed from other projects, neutron can also look in the future to remove the same , if possible . But till it is not, we can use "openstack extension list --network " to list network extensions, couldnt we ? | |
| 19:23:41 | dtroyer | I would also argue that we prefixed some resources with 'network' out of habit that didn't need it, again due to the misconception that "that is the way it should be done" rather than to fully-qualify the resource | |
| 19:24:49 | dtroyer | reedip_: yes, that would work. also, since no other API has extensions (in the OSC repo anyway) —network can be optional. if others appear then no option simply lists them all | |
| 19:24:49 | ankur-gupta-f4 | reedip_1: note the the command is extension show. List is already in place. | |
| 19:25:22 | dtroyer | why does a show command need a type identifier? | |
| 19:25:47 | dtroyer | to show something you have to have a name or ID to begin with? | |
| 19:26:04 | reedip_1 | sorry ankur-gupta-f4 , misread the command | |
| 19:26:18 | ankur-gupta-f4 | because list extensions exists for all core resources. But extension show only is a network command | |
| 19:26:39 | ankur-gupta-f4 | volume and compute APIs don't support the 'show' which returns more details about a specific API extension | |
| 19:27:13 | dtroyer | so either throw an exception for them (not found?) or return an empty or minimal result set | |
| 19:27:49 | sindhu | so i'll still keep in common ? | |
| 19:27:55 | dtroyer | yes | |
| 19:28:05 | ankur-gupta-f4 | but update help text plz | |
| 19:28:12 | ankur-gupta-f4 | as per John Davidge's comments | |
| 19:28:33 | sindhu | okay will do | |
| 19:28:45 | reedip_1 | dtroyer : but is keeping this implementation in common correct ? | |
| 19:29:13 | reedip_1 | I mean that was your query , and I found it right. Wondering what changed your mind :) | |
| 19:30:28 | dtroyer | phase of moon? | |
| 19:30:41 | reedip_1 | wont change till tomorrow :P | |
| 19:30:53 | dtroyer | I don't recall exactly what was the rationaly, but the way I understand it today may be different | |
| 19:31:24 | reedip_1 | so should we take this with a pinch of salt ??? | |
| 19:32:30 | ankur-gupta-f4 | moving on though. I want to bring up these updated functional test stuff coming in | |
| 19:32:47 | dtroyer | am I not allowed to change my mind if my understanding changes? | |
| 19:33:10 | reedip_1 | dtroyer : no you are , for sure :) | |
| 19:33:25 | dtroyer | also, given that a number of other commands have dependencies on knowing if extensions are installed/enabled, this is one area I would support putting into a common network lib (network.v2.common) | |
| 19:33:36 | reedip_1 | and I think keeping it in the network section sounds logical | |
| 19:33:38 | dtroyer | excpet that particular module already has command classes in it | |
| 19:33:44 | reedip_1 | ok | |
| 19:33:50 | dtroyer | err, network.common | |
| 19:33:59 | dtroyer | similar to what is in identity.common | |
| 19:34:25 | dtroyer | I'm talking about the actual access methods (calling REST) not the command classes | |
| 19:34:41 | dtroyer | so another command can easily check if an extension is enabled and act appropriately | |
| 19:35:34 | dtroyer | that doesn't need to be done immediately, but I wanted to mention it as an example of places I do think factoring out stuff is beneficial since I rant against that so much in the command classes | |
| 19:37:19 | ankur-gupta-f4 | makes sense. | |
| 19:38:26 | dtroyer | ok, so functional tests? | |
| 19:39:15 | ankur-gupta-f4 | have a patch like that but I want that to be the standard | |
| 19:39:15 | ankur-gupta-f4 | I would like to see more comprehensive tests specifically for some of the set/unset tests. Ive noticed a lot of them just set a description or something similar. I want to see them do a bit more. i.e. port command instead of just setting and unset description. I want them to create another resource like security group and set and unset the security group. I | |
| 19:39:51 | ankur-gupta-f4 | We can catch broken commands/resources faster that way | |
| 19:40:18 | dtroyer | exactly right | |
| 19:40:54 | ankur-gupta-f4 | okay. In that case Im going to start commenting on patches coming in that are still doing the superficial testing and do more thorough functional tests | |
| 19:41:11 | dtroyer | the mechanics of testing the option parsing belongs in unit tests, but especially where things interact with other resources we need to be checking deeper in functional tests | |
| 19:41:28 | dtroyer | good idea | |
| 19:41:58 | ankur-gupta-f4 | Okay. Sounds good. | |
| 19:42:18 | dtroyer | we don't need to duplicate unit tests, but some things also can be affected by changes in the underlying libs and this is the only place we catch those until we add more integration tests | |
| 19:43:09 | dtroyer | to be clear, I think we understand the scope of unit tests, and functional tests work against a running cloud | |
| 19:43:44 | dtroyer | what I'm calling integration tests (maybe the wrong name) test the stack from the command parser down the the requests session emitting HTTP | |
| 19:44:04 | dtroyer | so no actual server required, we mock the HTTP reply and look at the entire client stack | |
| 19:44:37 | ankur-gupta-f4 | hows the run time for something like that? | |
| 19:45:41 | ankur-gupta-f4 | just thinking beyond to bringing it up into voting job | |
| 19:45:55 | dtroyer | similar to unit tests. I have a few defined in tests.unit.integ. so far they are mostly for checking os-client-config behaviour | |
| 19:46:10 | dtroyer | they are run with the unit tests today | |
| 19:46:40 | dtroyer | where unit tests mock out things outside osc, these use the entire stack of dependencies down to the requests lib | |
| 19:47:06 | dtroyer | so osc-lib, python-*client (or SDK), os-client-config, etc | |
| 19:47:25 | ankur-gupta-f4 | neat will take a look | |
| 19:48:18 | dtroyer | hmmm… I'm looking now and it seems like I wrote more than what is there | |
| 19:49:58 | ankur-gupta-f4 | thats all I had for today assuming we are still holding off on networking commands for a while | |
| 19:50:55 | dtroyer | I think we're past where I'd worry about adding functionality, I started looking at some of those larger reviews again yesterday | |
| 19:51:23 | dtroyer | PTG next week will affect the flow, either better or worse, depending on the amount of hacking time we end up with | |
| 19:52:05 | ankur-gupta-f4 | right. PTG :( | |
| 19:53:03 | ankur-gupta-f4 | Flavor Profile and Flavor are two huge ones that could use some eyes | |
| 19:53:04 | dtroyer | ankur-gupta-f1: so just fly home on Sunday and get on the wrong place. oops! Hartsfield? that's not the right airport… oh well… | |
| 19:53:10 | dtroyer | s/place/plane | |
| 19:53:38 | ankur-gupta-f4 | haha. I was thinking of just 'missing' my flight back to Texas and being stuck in Portland forever :| | |
| 19:54:31 | ankur-gupta-f4 | alright. Thanks for the help Dean. Going to go grab some grub | |
| 19:54:39 | ankur-gupta-f4 | thanks reedip_1 : sindhu | |
| 19:54:45 | ankur-gupta-f4 | thanks for the help as well :) | |
| 20:41:11 | openstackgerrit | Brian Curtin proposed openstack/python-openstacksdk master: Implement message docs https://review.openstack.org/435104 | |
| 20:59:33 | openstackgerrit | Brian Curtin proposed openstack/python-openstacksdk master: Implement metric docs https://review.openstack.org/435110 | |