19:00:15 <briancurtin> #startmeeting python-openstacksdk
19:00:16 <openstack> Meeting started Tue Mar  3 19:00:15 2015 UTC and is due to finish in 60 minutes.  The chair is briancurtin. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:00:18 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
19:00:20 <openstack> The meeting name has been set to 'python_openstacksdk'
19:00:37 <briancurtin> if you're here for the SDK meeting, say hi
19:01:35 <terrylhowe> Terry Howe, HP
19:01:45 <stevelle> Steve Lewis, Rackspace
19:01:52 <briancurtin> Brian Curtin
19:02:05 <briancurtin> and Ian will be late, so that's our usual crew
19:02:20 <briancurtin> here's a little agenda https://wiki.openstack.org/wiki/Meetings/PythonOpenStackSDK#Agenda_for_2015-03-01_1900_UTC
19:02:50 <briancurtin> #topic conn.update vs conn.compute.update_server - https://review.openstack.org/#/c/160635/
19:02:53 <etoews> o/
19:03:27 <briancurtin> that topic, or ones like it, had come up in the past, but after working with the update_server call, i really like it, but also see the use case for conn.update (which i previously disliked)
19:05:07 <briancurtin> they support two different needs, with conn.update there if you want to update attributes on a resource and pass it in, or conn.compute.update_server (or really any update call on any resource in any service) being a way to accomplish the same thing via a function call
19:06:20 <briancurtin> the update_server way fits more closely with the way something like create_server works, so it looks pretty clean. i dont usually like providing multiple ways to do the same thing, but the conn.update method isn't something i think should have to go away, but it isn't something that we'd push as hard as the more resource-specific way to fit that mold
19:06:44 <terrylhowe> seems like most people would be updating a server through the properties rather than through a dictionary
19:07:02 <stevelle> +1, for update scenarios
19:07:24 <stevelle> thinking on it however
19:07:59 <briancurtin> terrylhowe: i think doing it via dictionary is probably the least likely case, but making use of the keyword arguments to pass in is super nice
19:09:08 <briancurtin> update_server(server, name="new name") is pretty hard to top, or in the case terry brought up that i broke, update_server(id="theid", name="new name")
19:10:13 <briancurtin> given how create_server(name, flavor, image) works, i'd like to allow update_server to look the same
19:10:29 <terrylhowe> server.name=“new name” #just seems like the more normal way to me
19:11:50 <briancurtin> another part of the reason i'd like to have update_server is that it allows you to stay within one context. doing your server operations on conn.compute.<something>_server feels nicer than having to jump from methods on conn.compute to methods straight on conn
19:12:58 <briancurtin> the server_builder example looks a bit nicer with create and update operations both being on conn.compute. the diff between patchset 1 and 2 shows how it changed
19:13:17 <stevelle> how about update_by(name=None, id=None, **data) for the alternate form?
19:13:30 <terrylhowe> we could move the generic updates, create etc to the base proxy class
19:13:32 <briancurtin> what does  update_by mean?
19:13:57 <terrylhowe> by_id maybe like the resource class
19:14:17 <briancurtin> that's too low level - you could just use the resource class directly then
19:15:00 <stevelle> I guess my thinking is that Terry's syntax could be supported by  that means
19:15:17 <stevelle> by_id("theid", name="new name") or the like
19:15:21 <briancurtin> just so my thought process is out there, i think the *main* usage is in the methods on a service, so compute.create_server, compute.delete_server, compute.update_server, etc. conn.create, conn.delete, conn.update, and the like are what i consider the alternate form
19:16:28 <stevelle> if you have a server and want to update it, use update_server.  If you don't, use the other form and specify an identifier
19:17:03 <stevelle> the name I suggested was just to allow update_by(id="theid", [...])
19:17:55 <briancurtin> i just dont understand the name update_by. the update is done on an ID, and the _by name seems confusing. it kind of evokes a SQL feeling and doesn't seem to fit in the higher level
19:18:30 <sigmavirus24> hm
19:18:33 <stevelle> the name is not important for me.
19:18:36 * sigmavirus24 scrolls backwards
19:18:48 <briancurtin> even if you dont have the server object, update_server(id=1, name="new") will cover that
19:19:15 <briancurtin> (right now i build off a passed in server obj, but i'm going to change that to default to none so that previous message works)
19:19:18 <terrylhowe> maybe if update_server just didn’t take **data instead
19:19:50 <briancurtin> taking **data is the best thing about it though
19:19:50 <terrylhowe> it would be kind of repetitive to what is out there, but more ituitive for the user
19:20:13 <terrylhowe> :)
19:20:41 <stevelle> sorry I'm thinking through the alternatives at the expense of the group's time
19:20:43 <briancurtin> if it takes **data is just takes the attributes on the resource. i'm liking this way a ton more than what i did for object_store, as in i'm thinking about making a temporary object_store2 that fits more with how compute is done so we can see side-by-side how it works
19:21:50 <briancurtin> i totally didn't really see how a lot of the current proxies, basically anything outside of object_store, were going to work, but after we got a bunch of stuff out of the way, they're more powerful than object_store's manual stuff
19:23:10 * sigmavirus24 isn't familiar enough with all of the proxies to be helpful here
19:23:51 <terrylhowe> to me the **data makes it look like the correct way to update an object is by passing in name values
19:24:18 <terrylhowe> there is also the problem of we have no method to create a new server without calling GET right now
19:24:41 <briancurtin> what's wrong with create_server?
19:24:54 <terrylhowe> that doesn’t help for a server that exists
19:25:15 <terrylhowe> I would just think some people would want to update an object without a call to GET
19:25:33 <sigmavirus24> terrylhowe: I would think likewise
19:25:42 <briancurtin> i think i'm missing what we're talking about here
19:26:01 <sigmavirus24> but in reality, the connection pooling in requests makes the extra request/response time tie directly to bandwidth and how long it takes the server to respond
19:26:18 <sigmavirus24> briancurtin: I'm agreeing the general let's not have one method make two requests
19:26:27 <briancurtin> what method makes two requests?
19:26:28 <etoews> terrylhowe: can you throw out some example user code of what user's can (or can't) do?
19:26:29 <sigmavirus24> but I'm not familiar with the code terrylhowe is mentioning
19:26:42 <etoews> i think that would clear things up quickly
19:26:55 <terrylhowe> with this update_server you need a server
19:27:16 <terrylhowe> server = conn.compute.existing_server(‘id’:’foo’)
19:27:27 <terrylhowe> server.name = ‘bar’
19:27:38 <terrylhowe> conn.compute.update_server(server)
19:28:00 <terrylhowe> we’d need some method like existing to create one with no GET
19:28:23 <briancurtin> if you have the id, you could just do update_server(id="foo", name="bar") **after i make the change that's in the comment on the review
19:28:43 <terrylhowe> ah, the None default
19:28:45 <briancurtin> if you happen to actually have a server already that you've received through a GET, you can pass it directly in, but you shouldn't need to
19:29:21 <briancurtin> terrylhowe: yep - if it's none, you'll have to pass in the ID (no way around it), but you won't need to GET just to have an object to pass in
19:31:06 <briancurtin> i'm currently working on some API guidelines as i'm writing some stuff, and i'll put together a doc that includes all of the possible ways you can do these operations and then see if/where we need to coalesce or expand
19:31:44 <sigmavirus24> ah
19:31:47 <sigmavirus24> makes sense
19:32:48 <briancurtin> since we spent a half hour on that, i'll write up more of the doc and share it and we can discuss further with more concrete examples
19:32:53 <stevelle> briancurtin: in cases where patch is not supported will that work uniformly across different service APIs?
19:33:43 <briancurtin> #topic put vs patch
19:34:34 <briancurtin> stevelle: we currently have to have resources specify if they use PUT for updates, defaulting to PATCH. from what i can tell, we should maybe switch that, so PATCH users have to specify instead of the more common PUT having to specify
19:35:59 <briancurtin> so your conn.update or conn.service.resource_update will work however you've configured the resource you're operating on
19:36:27 <terrylhowe> that might be for the best, not sure how many support PATCH.  maybe just open a ticket to investigate
19:36:34 <terrylhowe> if there isn’t one already
19:36:37 <briancurtin> i opened one last night
19:36:49 <etoews> there's a handful that use PATCH and more using it all the time
19:36:53 <briancurtin> PATCH shows up a few times in identity v3 and then once in another service i can't think of
19:37:00 <etoews> glance
19:37:09 <etoews> poppy
19:37:11 <etoews> zaqar
19:37:26 <briancurtin> etoews: what is more popular: put or patch
19:37:29 <etoews> put
19:37:38 <terrylhowe> encyclopedia etoews !
19:38:43 <briancurtin> etoews: is there an overall trend towards patch, or is this just newer projects wanting to use it? currently we get patch by default but have to specify for put, which currently feels backwards
19:38:46 <terrylhowe> anyway, research done.  PUT should be default
19:39:38 <sigmavirus24> briancurtin: I expect the API WG to suggest PATCH for updates (if we don't already have a guideline along those lines)
19:39:39 <etoews> i feel like there's a trend towards PATCH but can't quantify or even qualify this.
19:39:50 <etoews> sigmavirus24: +1
19:40:05 <sigmavirus24> etoews: we're everywhere lately ;)
19:40:51 <stevelle> I feel like expressing the resource's expectation in terms of PATCH is more meaningful and clear.
19:41:00 <stevelle> even if default is False
19:41:34 <briancurtin> i'm sitll with terrylhowe on PUT being the default just given reality of what we have to live with now and over the next while
19:42:07 <sigmavirus24> So inspite of what new APIs should be doing
19:42:13 <sigmavirus24> For now, it makes sense to default to PUT
19:42:32 <sigmavirus24> I garee with Brian and Terry since we're trying to provide the best interface to those APIs
19:42:34 <stevelle> default clearly should point at PUT, either way.
19:42:47 <sigmavirus24> I want to suggest an attribute like 'update_method' that defaults to PUT
19:42:56 <sigmavirus24> Because I think booleans are tricky here
19:43:11 <sigmavirus24> But I can see if we don't want to accept put/PUT patch/PATCH or whatever else for a resource
19:43:39 <briancurtin> sigmavirus24: i dont know how that will work with how we do the actual Resource.update
19:44:06 <sigmavirus24> Yeah I have to look but if we used session.request we'd be able to pass it directly as the first argument, no conditionals necessary
19:44:25 <sigmavirus24> briancurtin: just an idea, it doesn't change what the default behaviour should be though
19:44:26 <briancurtin> i think short term we should just flip that default, but explore how to make it better after that
19:44:33 <sigmavirus24> briancurtin: +1
19:44:40 <briancurtin> #action fix https://bugs.launchpad.net/python-openstacksdk/+bug/1427479 to flip the default from patch to put
19:44:41 <openstack> Launchpad bug 1427479 in OpenStack SDK "switch put_update to be patch_update" [Medium,New]
19:45:13 <briancurtin> #topic need to implement CaseInsensitiveDict for header dict
19:45:39 <briancurtin> that's sort of a note for me - we implemented the CID, then added the separate header storage, but never used the CID for it
19:46:01 <terrylhowe> should be easy to do
19:46:38 <briancurtin> #action finish https://bugs.launchpad.net/python-openstacksdk/+bug/1422234 to combine the resource.header storage with the CaseInsensitiveDict
19:46:40 <openstack> Launchpad bug 1422234 in OpenStack SDK "Case sensitivity in resource.prop doesn't work with headers" [Critical,New] - Assigned to Brian Curtin (brian.curtin)
19:46:53 <briancurtin> (i'm trying to get better at actually using meetbot)
19:47:07 <briancurtin> #topic identity: use set() for valid_options https://review.openstack.org/#/c/160840/
19:47:26 <briancurtin> some details aside, it seems like using a set here is actually fine? (i jumped in with something on KSC that i forgot is not true)
19:48:09 <stevelle> agreed
19:48:27 <terrylhowe> it makes sense to me, I think the change is good
19:48:34 * sigmavirus24 agrees as well
19:48:38 <briancurtin> cool, will take another look after this
19:48:40 <terrylhowe> I didn’t understand the perf implications
19:48:47 <sigmavirus24> terrylhowe: which ones?
19:48:54 <sigmavirus24> the ones in the most recent patch?
19:49:00 <terrylhowe> there was some mention in a comment
19:49:14 <sigmavirus24> it's a matter of convertiing a set to a list and adding multiple lists together and then converting back to a set
19:49:27 <terrylhowe> ah
19:49:29 <sigmavirus24> so creating 4 linked lists + a 5th from the previous 4th is a performance drag
19:49:41 <sigmavirus24> I provided an alternative to the rather ugly first attempt
19:49:58 <sigmavirus24> But instead of using the alternative that i proposed, Julien chose to use this new approach
19:50:18 <sigmavirus24> sets do make more sense and if we do any kind of opt in valid_options will make that much faster
19:50:24 <sigmavirus24> __contains__ for lists are O(n)
19:50:29 <briancurtin> given the tiny size, i dont think it's something to even worry about, really - readability matters more than there speed (and correctness matters, too)
19:50:53 <etoews> briancurtin: +1
19:51:02 <sigmavirus24> briancurtin: _union(A.valid_options, B.valid_options, C.valid_options) is both readable and fast ;)
19:51:22 <briancurtin> otherwise i'd like to see benchmarks if we're going to make changes to actually optimize code for performance
19:51:38 <terrylhowe> the code is only used during auth
19:52:03 <briancurtin> yep, it's really not a big deal, so as long as it's readable and correct, it's good
19:52:05 <sigmavirus24> And only executed when the class definition is loaded by Python
19:52:10 <sigmavirus24> it's a one time hit so it isn't a problem
19:52:35 <terrylhowe> sigmavirus24: suggest was good, but what is out there is probably good enough
19:52:36 <sigmavirus24> if it happened every time auth was instantiated it might be a problem then
19:52:51 <briancurtin> telling people you want to see benchmarks usually results in them realizing it's not actually important for performance
19:52:52 <sigmavirus24> terrylhowe: I'm just joking. I don't care if Julien listens to me
19:53:28 <terrylhowe> well, green light that
19:53:47 <briancurtin> will take a look in a few minutes
19:53:58 <briancurtin> anything else to cover in the last few minutes?
19:54:53 <terrylhowe> the tenant_id one I’d just like to try it out and make sure it works as expected
19:55:11 <terrylhowe> https://review.openstack.org/#/c/160821/2
19:55:32 <briancurtin> yeah i'll try that out as well
19:59:20 <briancurtin> thanks everyone!
19:59:28 <briancurtin> #endmeeting