13:00:38 <alex_xu> #startmeeting nova api
13:00:39 <openstack> Meeting started Wed Jul 26 13:00:38 2017 UTC and is due to finish in 60 minutes.  The chair is alex_xu. Information about MeetBot at http://wiki.debian.org/MeetBot.
13:00:40 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
13:00:42 <openstack> The meeting name has been set to 'nova_api'
13:00:47 <mriedem> o/
13:00:51 <alex_xu> who is here today?
13:00:53 <gmann_> o/
13:02:03 <alex_xu> ok, let us start the meeting :)
13:02:45 <alex_xu> I guess we have no more microversions can be merged in these two days?
13:03:00 <alex_xu> there still have few client side patches
13:03:07 <alex_xu> #link https://review.openstack.org/#/c/485435/5
13:03:17 <alex_xu> ^ start from this one
13:03:37 <alex_xu> mriedem: the client won't be freeze at tomorrow, right?
13:04:05 <mriedem> it is
13:04:05 <gmann_> 27 is final release right
13:04:08 * johnthetubaguy wonders in several months late
13:04:10 <mriedem> tomorrow is final client release
13:04:33 <mriedem> alex_xu: i just pushed up the test impl for that service delete test in the client patch
13:04:41 <gmann_> johnthetubaguy: :) welcome back
13:04:48 <alex_xu> johnthetubaguy: welcome back :)
13:05:00 <alex_xu> mriedem: ok, we need to speed up the review
13:05:18 <mriedem> alex_xu: yeah if you can go over it again before you end your day,
13:05:25 <mriedem> then i can have sdague take a look later today again too
13:05:38 <mriedem> or talk to andrey or Vek
13:05:43 <alex_xu> mriedem: yea, I will do that after the meeting
13:05:55 <mriedem> thanks
13:06:24 <alex_xu> for no more extension, I only expect this patch merge https://review.openstack.org/#/c/486414/2
13:06:34 <alex_xu> other cleanup can be done in next release
13:07:04 <alex_xu> mriedem: this one need +w https://review.openstack.org/#/c/485061/, then we finish the goal of no more extension in pike
13:07:16 <sdague> sorry, just wandering in
13:08:19 <sdague> alex_xu: nice!
13:08:20 <alex_xu> sdague: thanks :)
13:09:29 <alex_xu> I want to remove the objects related to extension, but I still found a lot of garbage in our unittests. so I think we can continue those in next release
13:10:01 <mriedem> alex_xu: looks like https://review.openstack.org/#/c/486416/ is the only other thing in that series that needs a +W
13:10:09 <mriedem> but the series needs to be rebased in order
13:10:51 <gmann_> alex_xu: yea, we do not have anything in functional tests
13:10:55 <mriedem> well maybe it'll go
13:11:00 <mriedem> i'll just rebase today as needed
13:11:14 <mriedem> do we have release notes for any of this at a high level
13:11:24 <mriedem> like, how is this different from when extensions were 'disabled' in newton?
13:11:35 <mriedem> because i know some people.....that have api extensions
13:11:48 <sdague> mriedem: the plumbing for extensions is now removed
13:12:03 <sdague> honestly, I don't think it's release note material
13:12:20 <sdague> this is internals that we said were not to be used a couple of releases ago, that are now deleted
13:12:29 <mriedem> but if i have an extension in my forked nova code repo, don't i just now register the route like everything else in here?
13:12:57 <sdague> mriedem: we used to provide infrastructure to let you do that in the system
13:13:02 <mriedem> i guess the stevedore extension stuff is gone,
13:13:06 <sdague> right
13:13:08 <mriedem> so my api extension could be a separate git repo
13:13:15 <sdague> or easily maintained
13:13:18 <mriedem> ok, yeah, i was thinking more about people that just have them in a forked nova branch
13:14:03 <sdague> if you have forked nova code... this is basically going to be a giant amount of work for you to get through
13:14:16 <sdague> but, that's never supposed to have been our target audience
13:15:01 <mriedem> still seems like it should be a release note
13:15:19 <mriedem> i don't expect people to keep all of the newton release notes in mind when reading the pike release notes
13:15:29 <mriedem> especially if they just installed openstack for the first time in ocata
13:15:36 <sdague> do we release note internals changes to objects?
13:15:53 <mriedem> no, but we release note the removal of deprecation options and hook points
13:15:55 <sdague> if they installed for the first time in ocata, this doesn't impact them
13:15:59 <alex_xu> we have release note for the extension related configure options
13:16:23 <sdague> mriedem: the docs were all scrubbed of that a while ago
13:16:45 <mriedem> alex_xu: this: "Deprecated config options to enable/disable extensions extensions_blacklist and extensions_whitelist have been removed. This means all API extensions are always enabled. If you modifed policy, please double check you have the correct policy settings for all APIs."
13:17:11 <alex_xu> mriedem: yea
13:17:24 <sdague> it's honestly going to be more confusing to release note this because it will say "we removed the ability to side load extensions through setup.cfg, that was never documented, and was officially removed 2 releases ago."
13:17:26 <mriedem> ok, and there is one about discoverable policy for extensions too
13:17:57 <mriedem> the API plugin stuff was documented in the devref before
13:18:08 <mriedem> or was that just bad naming
13:18:29 <mriedem> https://docs.openstack.org/nova/newton/api_plugins.html
13:18:51 <alex_xu> yea
13:18:53 <alex_xu> https://docs.openstack.org/nova/latest/contributor/api.html
13:19:23 <alex_xu> we still have something to update
13:19:59 <gmann_> yea those doc we can update
13:20:07 <mriedem> but, on the plus side,
13:20:15 <mriedem> there is nothing in there about stevedore loading out of tree API plugins/extensions
13:20:19 <mriedem> that's what i was looking for
13:20:35 <mriedem> so i think these docs are basically ok, they are just talking about how to add new APIs into the compute API
13:20:42 <mriedem> they need to be updated for the route map
13:20:46 <mriedem> but that's probably about it?
13:21:00 <alex_xu> yes
13:21:02 <gmann_> yea.
13:21:04 <mriedem> ok cool
13:21:09 <sdague> mriedem: yeh, definitely should be updated with the route map
13:21:09 <mriedem> let's move on then
13:21:10 <alex_xu> remove the 'extensions.V3APIExtensionBase'
13:21:31 <sdague> alex_xu: oh... interesting thing I found in running tests on the request logger
13:21:40 <gmann_> should we delete this also - https://docs.openstack.org/nova/latest/reference/stable-api.html
13:21:51 <gmann_> this talks about old extension background
13:21:57 <alex_xu> sdague: still have interesting thing happened?
13:22:03 <sdague> the handler for / requests inherits from the v2.0 legacy handler
13:22:10 <sdague> which means that microversions are stripped
13:22:31 <sdague> which, might be something that we should change
13:23:17 <alex_xu> gmann_: I need to reread those doc, if they just say something changed in the compatible v2.0 legacy API, that will be ok
13:23:18 <mriedem> gmann_: i wouldn't remove it, but i'd update it to say the ability to load API extensions via stevedore and blacklist/whitelist them is gone in pike
13:23:19 <sdague> it mostly just made writing the tests a little hard
13:23:19 <gmann_> this one ? https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/versions.py#L75
13:23:28 <mriedem> gmann_: that doc is mostly history
13:23:32 <sdague> gmann_: yes
13:23:34 <mriedem> so let's just make it current
13:24:02 <gmann_> mriedem: ok, make sense
13:25:16 <alex_xu> sdague: you mean no microversion info in the log when request to '/'?
13:26:46 <sdague> alex_xu: correct
13:26:59 <gmann_> sdague: alex_xu we have for v2.1 also - https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/versionsV21.py#L24
13:27:02 <sdague> but that also means it's not possible to do mv negotiation on that endpoint
13:27:17 <gmann_> https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/routes.py#L412-L413
13:27:37 <sdague> which, with some of mordred's stuff on standardizing version docs, might be appropriate
13:27:41 <sdague> just a thought
13:28:08 <sdague> we still need this deprecation in - https://review.openstack.org/#/c/486623/
13:28:59 <alex_xu> I think the python-novaclient do mv negotiation on the '/' now
13:30:28 <mriedem> right, using https://developer.openstack.org/api-ref/compute/?expanded=list-all-major-versions-detail#list-all-major-versions
13:30:33 <mriedem> you get back the min/max versions
13:30:45 <mriedem> to determine if you can do 2.53 or just 2.20
13:31:03 <mriedem> do we need microversion negotiation for handling a 2.1 vs 2.53 request to / ?
13:31:07 <mriedem> the response is the same isn't it?
13:32:43 <alex_xu> mriedem: yes, it is same
13:33:05 <mriedem> ok, so doesn't seem like a major issue right now
13:33:11 <sdague> yeh, it was more of an fyi
13:33:32 <alex_xu> ok
13:33:59 <mriedem> so to recap,
13:34:05 <mriedem> 1. novaclient 2.53 change needs review
13:34:19 <mriedem> 2. remaining no more api extensions patches are approved, we just need to shephard through the gate
13:34:29 <mriedem> 3. need to update the api plugins / stable api docs for the route map stuff
13:34:39 <mriedem> 4. deprecate wsgi_log_format
13:34:54 <mriedem> yeah?
13:35:14 <alex_xu> yea
13:35:43 <gmann_> yes.
13:35:44 <alex_xu> and one bug from Kevin_Zheng https://review.openstack.org/486850
13:36:19 <alex_xu> only #1 and #2 need to be done before freeze
13:36:39 <Kevin_Zheng> o/
13:36:39 <gmann_> is href so useful ? can we just have base url with version
13:36:57 <gmann_> i can do 3.
13:37:02 <alex_xu> gmann_: that sounds like a API behaviour change
13:37:12 <alex_xu> gmann_: thanks
13:37:16 <gmann_> alex_xu: humm
13:37:37 <alex_xu> I think we can get correct url by 'req.environ['PATH_INFO']'
13:37:45 <alex_xu> Kevin_Zheng: worth a try ^
13:38:18 <gmann_> alex_xu: but if we put some random string there.
13:38:30 <gmann_> i tried like GET http://10.76.150.17/compute/xyz
13:38:52 <gmann_> then how we know what is actual endpoint patch till 'compute' or /
13:39:04 <gmann_> patch->path
13:40:50 <alex_xu> gmann_: I see 'PATH_INFO' will return 'xyz'
13:41:29 <gmann_> yea
13:41:47 <Kevin_Zheng> we should also filter /compute
13:42:05 <Kevin_Zheng> or anything was set to be the endpoint
13:42:15 <gmann_> Kevin_Zheng: /compute is not all time thing. it can be anything
13:42:22 <alex_xu> yea
13:42:25 <gmann_> so we left with base url
13:42:45 <sdague> We should probably actually reach back into the keystone catalog to get the right value
13:42:49 <sdague> that's where it's really stored
13:42:52 <Kevin_Zheng> I understand, I mean, path will also include this
13:43:00 <Kevin_Zheng> and it will got duplicated
13:43:09 <gmann_> after that what is wrong version coming for / request is not known to be bad version or bad string
13:43:10 <sdague> that being said, this seems like a pretty minor bug, right?
13:43:40 <sdague> oh, I guess we're returning more garbage than I was expecting
13:43:42 <gmann_> sdague: for me too, i never read href :)
13:43:57 <sdague> yeh, we do document it, I don't think anyone uses it
13:44:19 <sdague> the real issue is the 300 multiple choice docs
13:44:27 <sdague> those aren't really useful
13:44:52 <alex_xu> yea
13:45:06 <mriedem> always pick "C" if you don't know the answer
13:45:19 <alex_xu> but I still think req.application_url + version + req.environ['PATH_INFO'] is the correct url
13:45:28 <sdague> so... I do wonder if the real fix is related to the routes
13:45:52 <sdague> because now that we've got all the routes in the list, we could say "oh, this thing you are trying to get looks like a thing we know about, but in a subtree"
13:46:06 <alex_xu> oh...
13:46:11 <sdague> because right now that 300 multiple choice selector just reflects you the garbage you send in
13:46:25 <sdague> which might also be non existant
13:46:43 <sdague> it would be really nice if it only reflected the 300 if the guessed routes were a thing
13:46:48 <sdague> otherwise, 404
13:47:41 <alex_xu> yea, that sounds good, but we still need to fix that url when the routes were a thing :)
13:49:40 <mriedem> Kevin_Zheng said other services just return a 404
13:49:56 <mriedem> i think i reported a bug against glance's API for this same issue, trying to get versions using /
13:49:59 <Kevin_Zheng> I tried Cinder Neutron
13:50:08 <sdague> Kevin_Zheng: they are using pecan, right?
13:50:23 <sdague> that probably saves them a bit here. All the nova code to do this is really custom
13:50:29 <sdague> I don't think they do the 300 docs
13:50:41 <sdague> as it wasn't a construct you could get into pecan
13:50:49 <mriedem> https://bugs.launchpad.net/glance/+bug/1632742
13:50:49 <openstack> Launchpad bug 1632742 in Glance "/v2 route doesn't exist" [Undecided,Won't fix]
13:51:16 <mriedem> not exactly the same
13:51:45 <sdague> yeh
13:53:19 <alex_xu> https://github.com/openstack/nova/blob/master/doc/api_samples/servers/server-get-resp.json#L43
13:53:28 <alex_xu> ^ I guess that is why we return 300
13:53:33 <alex_xu> for the bookmark url
13:54:20 <sdague> yeh
13:54:30 <sdague> which has lots of issues with it
13:54:45 <sdague> but, regardless, if we could test the route values before we return them in our route list
13:54:53 <sdague> then 404 if they don't exist
13:54:57 <sdague> that would probably be best
13:55:27 <alex_xu> ok, I'm ok that without microversion
13:56:49 <gmann_> sdague: test route list means? with some checking in our routing mapping
13:57:00 <gmann_> route value
13:57:27 <alex_xu> 3 mins left
13:57:55 <alex_xu> I think that should be another patch
13:58:25 <alex_xu> one fix the url, another one checks the routes
13:59:10 <gmann_> k
13:59:21 <gmann_> 1 min more
13:59:44 <alex_xu> ok, let us move back to nova change or to the patch :)
13:59:58 <alex_xu> thanks all
14:00:00 <alex_xu> #endmeeting