openstackgerrit | Merged openstack-infra/shade: Don't fail on trying to delete non-existant images https://review.openstack.org/388702 | 00:33 |
---|---|---|
*** abregman has joined #openstack-shade | 06:06 | |
*** abregman has quit IRC | 07:59 | |
*** abregman has joined #openstack-shade | 08:03 | |
*** abregman has quit IRC | 08:27 | |
*** abregman has joined #openstack-shade | 08:36 | |
*** abregman is now known as abregman|mtg | 08:36 | |
*** abregman_ has joined #openstack-shade | 09:08 | |
*** abregman|mtg has quit IRC | 09:11 | |
openstackgerrit | Arie Bregman proposed openstack-infra/shade: Add compute usage support https://review.openstack.org/393873 | 11:22 |
openstackgerrit | Arie Bregman proposed openstack-infra/shade: Add compute usage support https://review.openstack.org/393873 | 12:23 |
*** abregman_ is now known as abregman|mtg | 13:08 | |
*** abregman|mtg is now known as abregman | 13:24 | |
*** wowbagger42 has joined #openstack-shade | 13:34 | |
*** openstackgerrit has quit IRC | 13:48 | |
*** openstackgerrit has joined #openstack-shade | 13:48 | |
*** abregman is now known as abregman|brb | 14:32 | |
*** abregman|brb is now known as abregman | 14:45 | |
*** edtubill has joined #openstack-shade | 15:04 | |
*** wowbagger42 has quit IRC | 15:37 | |
*** edtubill has quit IRC | 16:21 | |
*** edtubill has joined #openstack-shade | 16:28 | |
openstackgerrit | Monty Taylor proposed openstack/oaktree: Add a few more docs https://review.openstack.org/395076 | 16:39 |
*** abregman has quit IRC | 16:45 | |
*** edtubill has quit IRC | 17:30 | |
*** edtubill has joined #openstack-shade | 17:31 | |
mordred | Shrews, SpamapS: I just replied to https://review.openstack.org/#/c/393884 - could y'all check it out and see if you agree? | 18:16 |
* Shrews reading the novella | 18:19 | |
*** clarkb has joined #openstack-shade | 18:21 | |
mordred | Shrews: I also sent a quick summary to the infra list | 18:21 |
Shrews | mordred: SpamapS: so, shade has always tried as best it can to hide cloud differences, but i don't think we've ever promised to make everything the same, especially b/w nova and neutron clouds. we *definitely* haven't promised to implement functionality where lacking. I'm fine with the change as-is. | 18:22 |
*** abregman has joined #openstack-shade | 18:22 | |
Shrews | for example, shade would NEVER try to implement egress rules for a nova-based cloud | 18:23 |
mordred | yes. totally agree | 18:23 |
mordred | I think my main point is that we _can_ make the filtering interface the same for the user for both in this case, it's just that it'll be less efficient for nova-net which I'm personally fine with | 18:24 |
clarkb | that, plus its already transparent to the user via the interface you just get nasty fail surprise errors when it breaks which is badness | 18:24 |
clarkb | if it was an explicit "nova net here and neutron here" fine treat them different but that isn't how its setup, instead its a one size fits all "filter things for me" so we should make it work for both | 18:25 |
mordred | yah, but I'm also fine with it not happening this instant, because the change to add it later isn't going to break anyone, and the change itself doens't change behavior for current users- it only introduces a new feature that only works for half of the users so far | 18:25 |
Shrews | yeah, i don't think we've ever said you can always filter all the things, but we should try to as best we can. i'm fine with it being added piecemeal | 18:28 |
mordred | I'm more concerned about making sure that normalization and push-down conditions don't step on each other | 18:29 |
clarkb | Shrews: yes there isn't an explicit statement to that effect but the "fail hard with an exception if you don't do the right thing which is completely opaque to the user" is what i htink we should avoid | 18:29 |
clarkb | because if user moves around to different clouds (hey tahts me!) it is incredibly frustrating when I have to go debug failures that show up in 1 of 10 clouds | 18:29 |
*** abregman is now known as abregman|nb | 18:30 | |
mordred | totally. that's why I think the error needs to say "please use search_floating_ips instead of list_floating_ips with a filter for now" | 18:30 |
mordred | since search_floating_ips will _always_ work for all of the clouds today | 18:30 |
mordred | and it's only the list_floating_ips push-down case if explicitly triggered by an end user where there is an issue | 18:30 |
Shrews | why do we have filtering on "list_*" anyway? the original purpose of "list" apis was to list all of them, and "search" apis could filter | 18:31 |
Shrews | i'm not sure how the filtering in list* started... | 18:31 |
clarkb | ya mordred brought htat up in the email, basically they should be combined | 18:31 |
mordred | filtering on list is so that filters from search can be passed in to list_ | 18:31 |
mordred | because if we want to do server-side pushdowns, we have to pass them to list | 18:31 |
Shrews | hrm, we did that wrong | 18:32 |
mordred | which, now that we've gotten this far shows that actually there's no real difference between the two, since we have to plumb the filters into list anyway | 18:32 |
mordred | yah | 18:32 |
mordred | we didn't do this the right way for sure | 18:32 |
Shrews | damn you, hindsight | 18:33 |
*** abregman|nb has quit IRC | 18:41 | |
mordred | Shrews: biggest most important question to sort now - which of list_ or search_ do we want to be the 'real' api and which one do we want to be the alias? | 18:44 |
mordred | Shrews: I'd like for my shed to be a nice purple color | 18:44 |
Shrews | mordred: the obvious answer is to create new "query_*" apis and have the other two be aliases. Also, green. | 18:45 |
mordred | mmm. green | 18:45 |
jlk | mordred: funny, your name in my client is purple. | 18:45 |
Shrews | even funnier, in mine, he's green | 18:45 |
clarkb | I would go with list because that seems to be what a lot of other clients libs do | 18:45 |
clarkb | which will make life a little simpler for anyone moving from eg neutronclient/novaclient | 18:45 |
Shrews | mordred: so, not all list_'s filter, but all search_'s do. we'd have to change the list_'s to all filter before we could promote that | 18:46 |
Shrews | and clarkb ^^^ | 18:47 |
clarkb | oh thats a good point | 18:47 |
mordred | Shrews: yes. I mean, as long as the filter arguments are optional and don't filter anything when missing, it shoudl be a no-op change | 18:47 |
clarkb | maybe shade is sufficiently its own thing to say just do search then | 18:47 |
mordred | oh - but yeah. doy. I understand your words now | 18:48 |
Shrews | good, b/c i did not understand yours | 18:48 |
*** root4 is now known as olaph | 18:48 | |
Shrews | fwiw, i think list_ is the better choice, but requires the code change | 18:49 |
Shrews | then we'd deprecate the search_ apis for a few version | 18:50 |
SpamapS | clarkb: I understand the "don't fail on arbitrary cloud differences" desire. However, is running 1000x slower not also a failure? | 18:54 |
clarkb | SpamapS: not in this case where nova net already could be filtered client side | 18:54 |
clarkb | SpamapS: so I think that ship sailed? | 18:54 |
clarkb | so this is about making things consistent with what we already have | 18:55 |
clarkb | not introducing any new slowness | 18:55 |
mordred | also - to be fair to the optimization problem - nodepool does full lists on all things with no pushdowns - and we have a TON of resources on our clouds and it's actually a thing we do to make things faster | 18:55 |
clarkb | and if you do think thats an issue refactor it to make it abundantly clear that one thing is supported and the other is not | 18:55 |
mordred | so making nova-net fip filtering client-side only, while slower, actually shouldn't be noticable in most cases | 18:55 |
clarkb | like I siad my biggest concern is its all behind the covers and only a month after writing your code do you discover oh this is completely wrong | 18:56 |
SpamapS | so let me understand: does shade already filter fips? | 18:56 |
mordred | yes. client-side | 18:56 |
mordred | if you use the search_floating_ips method | 18:57 |
SpamapS | So the concern is that this new functionality only works for neutrons? | 18:58 |
clarkb | SpamapS: in a confusing manner because it works over here but not over there and there is no documentation or clear error message for that | 18:58 |
clarkb | SpamapS: so my suggestion to address that was why not just make it work since it already works over here | 18:59 |
clarkb | and in the process make the code much simpler | 18:59 |
clarkb | alternate method would be to beef up documentation and error messages | 18:59 |
SpamapS | I think landing it half-functional is ok. It's not _that_ confusing, but agreed, land docs or the full functionality. | 19:03 |
openstackgerrit | Clark Boylan proposed openstack-infra/shade: Add unit test to show herd protection in action https://review.openstack.org/392120 | 19:09 |
clarkb | ^ failed to merge because it depended on old patchset | 19:09 |
clarkb | rebased and reapproved | 19:09 |
mordred | clarkb: thanks! | 19:10 |
*** abregman has joined #openstack-shade | 19:55 | |
*** abregman has quit IRC | 20:49 | |
openstackgerrit | Merged openstack-infra/shade: Add unit test to show herd protection in action https://review.openstack.org/392120 | 20:49 |
openstackgerrit | Merged openstack-infra/shade: Allow server-side filtering of Neutron floating IPs https://review.openstack.org/393884 | 21:52 |
openstackgerrit | Merged openstack-infra/shade: Use floating-ip-by-router https://review.openstack.org/390004 | 21:52 |
openstackgerrit | Merged openstack-infra/shade: Update floating ip polling to account for DOWN status https://review.openstack.org/390014 | 21:52 |
openstackgerrit | Merged openstack-infra/shade: Refactor out the fallback-to-router logic https://review.openstack.org/394613 | 21:52 |
*** edtubill has quit IRC | 22:14 | |
openstackgerrit | Merged openstack/oaktree: Add a few more docs https://review.openstack.org/395076 | 22:34 |
openstackgerrit | Arie Bregman proposed openstack-infra/shade: Add support for limits https://review.openstack.org/395235 | 22:44 |
openstackgerrit | Monty Taylor proposed openstack/oaktree: Clean a couple of meaningless things https://review.openstack.org/395241 | 23:00 |
thingee | mordred the requirements repo has this set differently for hacking in test-requirements https://review.openstack.org/#/c/395241/ | 23:37 |
mordred | thingee: yah - there's a patch either up or coming to fix it | 23:40 |
mordred | thingee: or maybe I'm lying. there was a conversation in infra this morning about it being wrong/too loose | 23:41 |
openstackgerrit | Merged openstack/oaktree: Clean a couple of meaningless things https://review.openstack.org/395241 | 23:42 |
openstackgerrit | Monty Taylor proposed openstack/oaktree: Manual sync with global requirements https://review.openstack.org/395258 | 23:49 |
mordred | thingee: ^^ did a sync with g-r by hand | 23:49 |
thingee | mordred setup.py should be pbr>=1.6 instead of 1.8 ? | 23:53 |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!