09:00:38 <priteau> #startmeeting blazar
09:00:39 <openstack> Meeting started Tue Nov 27 09:00:38 2018 UTC and is due to finish in 60 minutes.  The chair is priteau. Information about MeetBot at http://wiki.debian.org/MeetBot.
09:00:40 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
09:00:42 <openstack> The meeting name has been set to 'blazar'
09:01:01 <priteau> #topic Rollcall
09:01:11 <masahito> o/
09:01:44 <bertys> o/
09:01:57 <priteau> Hi masahito and bertys
09:02:36 <priteau> tetsuro: Are you here too?
09:02:51 <tetsuro> o/
09:03:25 <priteau> We decided that today we would do some code review
09:03:37 <priteau> Let's do AOB first
09:03:43 <priteau> #topic AOB
09:04:05 <priteau> Anything to discuss before we start discussing reviews, which might take the full hour?
09:04:33 <priteau> As mentioned last week, we will cancel next week's meeting due to travel of masahito and tetsuro
09:06:00 <priteau> If nothing let's start with the reviews
09:06:05 <priteau> #topic Code review
09:07:44 <priteau> In openstack/blazar, the two main patch series are resource-availability-api (resource allocation blueprint) and placement
09:08:04 <masahito> sorry, I have to leave few mins. please start tetsuro's patches.
09:08:10 <priteau> Otherwise we have various small fixes.
09:08:34 <priteau> tetsuro has only one pending patch now, it's https://review.openstack.org/#/c/584744/
09:09:08 <tetsuro> I saw your comments, priteau. Will submit another patch set.
09:09:12 <priteau> I reviewed it yesterday, I think maybe a code path were we need to delete the inventory was missed, but I am not sure
09:09:32 <tetsuro> Your point is fair.
09:09:40 <tetsuro> Good catch. Thanks.
09:11:02 <priteau> OK. While masahito is away, let's look at smaller patches.
09:11:15 <masahito> I'm back :-)
09:11:21 <priteau> Ah great.
09:11:53 <priteau> We're looking at https://review.openstack.org/#/q/status:open+project:openstack/blazar+branch:master+topic:bp/resource-availability-api
09:12:34 <priteau> I reviewed it yesterday and, while the code approach is good, I have concerns about the format of the API response
09:12:36 <masahito> got it.
09:13:03 <masahito> The sample response is this: http://logs.openstack.org/72/584272/8/check/build-openstack-api-ref/69d2e28/html/v1/index.html?expanded=list-allocations-detail#list-allocations
09:13:05 <priteau> To summarize, the response looks like this: http://logs.openstack.org/72/584272/8/check/build-openstack-api-ref/69d2e28/html/v1/index.html?expanded=list-allocations-detail#list-allocations
09:14:22 <priteau> I am thinking it would make more sense for it to look like this: http://paste.openstack.org/show/736064/
09:15:04 <priteau> What changed: inverted "allocations" <-> "reservations", and changed the main "id" to "host_id"
09:16:03 <priteau> With my proposed API response format, when you look at the "reservations" list, the "id" fields are actually reservation IDs, so I think it makes more sense
09:17:31 <priteau> A more minor comment is that in the "Get Allocations" case (http://logs.openstack.org/72/584272/8/check/build-openstack-api-ref/69d2e28/html/v1/index.html?expanded=list-allocations-detail,get-allocations-detail#get-allocations), we do a GET /reservations (plural), but get back an object saying "reservation" (singular)
09:17:49 <masahito> And your suggest is changing the URI to /v1/oshost/allocations, right?
09:18:07 <priteau> Yes, change the endpoint name as well
09:18:16 <priteau> What do you think?
09:19:02 <masahito> Make sense to me.
09:20:11 <priteau> Any though on the naming of the host ID field? Could be host_id or computehost_id. I don't know if we have any existing reference in the API already.
09:20:21 <masahito> Ah, AFAIK you have proposed another allocation API for USER API at Denver PTG?
09:21:33 <priteau> Are you talking about the https://blueprints.launchpad.net/blazar/+spec/query-reservation-candidates blueprint?
09:21:46 <masahito> To match the host APIs, what about just "host" for the key?
09:22:40 <priteau> "host" could work as well.
09:23:01 <priteau> In the host API we never mention the "compute" part, so we shouldn't use "compute" actually.
09:23:21 <masahito> No,  this blueprint https://blueprints.launchpad.net/blazar/+spec/reservation-consumers-api
09:24:29 <priteau> reservation-consumers would be under the lease API endpoint
09:24:54 <masahito> This bp uses consumers not allocation. Never mind, it was my fault.
09:25:01 <priteau> I need to spec it. I don't think it is related though.
09:26:39 <priteau> If you're happy with the proposed change of API response, would you be OK updating your patches? The rest of the code looks good so I can approve quickly.
09:26:53 <masahito> Of course.
09:27:03 <priteau> tetsuro, bertys: any comment about this?
09:28:15 <masahito> priteau: Now that I'm thinking about it, 'host' or 'host_id' is not better because it can't be applied to network resource.
09:29:25 <priteau> Right. It's under /os-hosts though. But we can make it abstract enough so that the same client code can be used to parse the results.
09:29:32 <priteau> resource_id?
09:29:55 <masahito> Great idea!
09:30:28 <priteau> With API documentation explaining that in this case, resource_id == host ID
09:30:55 <masahito> It really make sense.
09:31:20 <priteau> OK, I think we are in aggreement.
09:31:32 <priteau> *agreement*
09:33:12 <masahito> 1. change the endpoint to allocations, 2. follow the response body to http://paste.openstack.org/show/736064/ except 'host_id', and 3. use resource_id instead of host_id in the body.
09:35:28 <priteau> There's also the issue of "allocations" vs "allocation" in the "Get allocations on a host." case. I am unsure what is the best approach for this.
09:36:16 <priteau> Maybe it's the endpoint that should be v1/os-hosts/{host_id}/allocation?
09:36:56 <masahito> It looks like there is only one allocation on the host.
09:37:52 <masahito> Another idea is removing 'resource_id' key from the response because host_id is already in the URI.
09:39:43 <masahito> like this http://paste.openstack.org/show/736065/
09:40:45 <priteau> It's a tricky one. The way you've originally proposed it, it looks similar to the lease and host APIs.
09:41:45 <masahito> that's true...
09:42:42 <priteau> I guess it could be considered as one allocation of a hosts to many reservations.
09:44:23 <priteau> bertys: tetsuro: any thoughts on this API?
09:46:21 <priteau> If you're happy to change the endpoint to v1/os-hosts/{host_id}/allocation (since it would return a single allocation dictionary), let's do that
09:46:41 <priteau> Only 15 minutes left in the meeting so we should discuss other patches.
09:47:30 <priteau> List of Blazar master patches that can be merged: https://review.openstack.org/#/q/project:openstack/blazar+is:open+label:verified+branch:master+is:mergeable
09:48:24 <priteau> Easy one: https://review.openstack.org/#/c/620136/
09:48:51 <priteau> Noticed it yesterday in logs, we've moved the option in config but not updated the DevStack plugin
09:49:46 <masahito> Thank. LGTM. If others doesn't have objections, I merge it.
09:51:05 <priteau> tetsuro: Is this something that you are implementing already? https://review.openstack.org/#/c/578641/
09:51:46 <priteau> masahito: There's a +2 from bertys already, go ahead.
09:51:56 <tetsuro> No I've not yet started this spec.
09:52:37 <priteau> tetsuro: Is the spec still compatible with your approach, i.e. would you give it a +1?
09:52:45 <priteau> I haven't looked at it in a long time
09:52:54 <tetsuro> I'll give it a +1
09:53:15 <tetsuro> this is compatible with what I'm doing now.
09:53:55 <priteau> OK, great. I will give it another look as well
09:54:16 <priteau> Mutable config patch: https://review.openstack.org/#/c/585847/
09:54:36 <priteau> I tested it yesterday and found issues, but now realize that the issues already exist without the patch
09:55:01 <priteau> blazar-manager doesn't like receiving SIGHUP. Maybe an eventlet issue?
09:55:06 <priteau> I will open a Launchpad bug.
09:56:18 <priteau> Another easy patch, just to clear the queue: https://review.openstack.org/#/c/619464/
09:56:37 <masahito> I guess RPCServer in blazar.utils.service should need to have SIGHUP signal handler.
10:00:16 <priteau> OK, thanks for the pointer. We will need to look into it. In the meantime I think we can merge the mutable config patch.
10:00:53 <priteau> We're running out of time. I think most patches now have a -1 with action to update them, so we made good progress.
10:01:26 <masahito> priteau: I'll update the floating ip spec in this week. please review it.
10:01:33 <priteau> Thanks masahito
10:01:38 <priteau> No meeting next week, then code review again the week after (December 11)
10:01:48 <priteau> Thanks everyone!
10:01:55 <priteau> Have a good trip
10:01:57 <priteau> #endmeeting