14:00:32 <jokke_> #startmeeting glance
14:00:33 <openstack> Meeting started Thu Mar 29 14:00:32 2018 UTC and is due to finish in 60 minutes.  The chair is jokke_. Information about MeetBot at http://wiki.debian.org/MeetBot.
14:00:34 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
14:00:36 <openstack> The meeting name has been set to 'glance'
14:00:40 <jokke_> #topic roll call
14:00:45 <abhishekk> o/
14:00:45 <jokke_> o/
14:00:50 <rosmaita> o/
14:00:53 <rosmaita> smcginnis too
14:01:00 <jokke_> #link https://etherpad.openstack.org/p/glance-team-meeting-agenda
14:01:29 <smcginnis> o/ o/ :)
14:01:43 <rosmaita> \o
14:01:58 <McClymontS> o/
14:02:16 <jokke_> #topic updates
14:03:24 <jokke_> so pretty muc one update ... trying to find the mail
14:04:08 <jokke_> but in  nutshell we're moving from maintaining the minimum requirements centrally to in-repo requirement management
14:05:19 <jokke_> and there will be similar lower-constrains job as there is upper constrains
14:05:45 <rosmaita> http://lists.openstack.org/pipermail/openstack-dev/2018-March/128352.html
14:05:51 <McClymontS> Saw that patch to be reviewed
14:05:54 <McClymontS> for the lower constraints
14:05:59 <rosmaita> https://review.openstack.org/#/c/555483/
14:06:28 <jokke_> dhellmann did script the needed changes, so I'd like everyone to get familiar with how it affects us, and what pitfall we need to look out for
14:06:35 <jokke_> rosmaita: thanks
14:06:41 <rosmaita> np
14:07:00 <jokke_> #link  http://lists.openstack.org/pipermail/openstack-dev/2018-March/128352.html
14:07:26 <rosmaita> that's a fairly long email!
14:07:49 <jokke_> other update is that we still haven't got the glanceclient released so lets hop into topic 2
14:08:09 <jokke_> #topic python-glanceclient image-import default behavior
14:08:36 <jokke_> rosmaita: you wanna start on this?
14:08:50 <rosmaita> sure
14:08:52 <rosmaita> #link https://review.openstack.org/#/c/555550/
14:09:08 <rosmaita> ok, so i was testing out glanceclient and came across some situations
14:09:29 <rosmaita> decided to handle them by doing more analysis on the client side to see if a request was likely to succeed before making it
14:09:47 <rosmaita> because the problem is that you can have client-side import succes, that is the import call succeeds
14:09:57 <rosmaita> but hte actual import fails later when the task is processed
14:10:02 <rosmaita> and that can be problematic
14:10:03 <rosmaita> anyway
14:10:35 <rosmaita> as part of that, i changed the default import-method to none, and in that case all the image-create-via-import does is create an image record
14:10:50 <rosmaita> or exits if you have included extra junk
14:11:02 <rosmaita> so, the question is whether that is a good change
14:11:22 <rosmaita> my reasoning is that we have >1 import method, so the user should pick
14:11:43 <rosmaita> but, that is backward incompatable with the current image-create command
14:12:00 <rosmaita> assuming that we will eventually replace image-create with image-create-via-import
14:12:15 <rosmaita> but, it might be worth doing anyway
14:12:30 <rosmaita> anyway, that's the basic idea
14:12:50 <rosmaita> i personally can go either way, key thing is that we've got tests to make sure the input makes sense
14:13:14 <rosmaita> but obviously, i think making the user say what import-method they are using is a good idea
14:13:25 <rosmaita> since that's what my patch does
14:13:34 * rosmaita shuts up
14:13:50 <jokke_> so, I'm hugely afraid about the outcry if we replace the image create client call with something that is not backwards compatible ... on the same time as image-create-via-import is experimental, we can experiment with it, not rename it but rewrite the image-create with sensible defaults when the time comes to swap
14:15:09 <jokke_> that is our benefit of flagging it experimental in first place, and big thanks to rosmaita pointing out that we should not lift that flag just yet
14:15:40 <rosmaita> i guess the point of a decision now is that the openstack client people or sdk people may look at our implementation as a reference
14:16:33 <rosmaita> or maybe that's the point of having a non-backward-compat change in the experimental command
14:16:50 <McClymontS> +1
14:17:05 <rosmaita> i guess my point is that i don't like silent defaults with complex choices
14:17:17 <rosmaita> "explicit is better than implicit"
14:17:21 <rosmaita> i heard that somewhere
14:17:58 <jokke_> rosmaita: yeah, so it's flagged experimental so we can get it running, test it and gather feedback. Once we make what ever the call is how it's behaving and remove that experimental flag, we need to start to be really careful what we break
14:18:11 <rosmaita> jokke_ agree 100%
14:18:53 <rosmaita> my short-term question is do we like the current patch, or do i need to rewrite it?
14:22:08 <jokke_> so I'm on favor of having sensible defaults or at least ability to set them
14:22:28 <rosmaita> just a reminder: stable-maint team reviews release requests on mondays and we plan to release from stable/queens
14:23:02 <jokke_> and this coming Monday is easter so I quess we need to poke tonyb anyways
14:23:26 <rosmaita> does RH have monday off?
14:23:42 <jokke_> I have both tomorrow and Monday off
14:23:49 <rosmaita> slackers!
14:24:09 <abhishekk> I have tomorrow off
14:24:44 <rosmaita> i would suggest that a sensible default would do minimal damage, so in this instance, creating an image record only is a good choice
14:25:06 <rosmaita> but that's just me
14:25:17 <jokke_> well we can still do that if the payload is not provided
14:25:55 <jokke_> but sensible default would be something that maintains backwards compatibility to established command
14:26:07 <jokke_> that's why it's there in the first place ;)
14:26:09 <rosmaita> i guess my concern there is if we have multiple methods that take --file, that's a problem
14:26:18 <rosmaita> maybe --uri is a better example
14:26:29 <rosmaita> already a proposal for another method for that
14:27:23 <jokke_> yeah, and the default that is in place is backwards compatible and should not accept --uri
14:27:53 <jokke_> but like said, I'd be fine giving the user opportunity to set the default locally as well
14:27:55 <rosmaita> well, let's take a vote or something ... i can make the changes right away and update all the tests if necessary ... looks like it needs to be done in the next few hours if we're going to propose a release for monday
14:28:19 <jokke_> having env variable for it, just like we have been doing with the api version
14:29:29 <rosmaita> that's a good idea
14:29:35 <McClymontS> I like that idea actually
14:29:44 <rosmaita> and you mentioned it before but i ignored it
14:29:56 <jokke_> #startvote image-create-via-import should have sensible defaults? Yes, No, users-choise
14:29:57 <openstack> Begin voting on: image-create-via-import should have sensible defaults? Valid vote options are Yes, No, users-choise.
14:29:58 <openstack> Vote using '#vote OPTION'. Only your last vote counts.
14:30:13 <jokke_> crap
14:30:14 <rosmaita> ummm ... we need to say what "sensible defaults" are!
14:30:17 <jokke_> #endvote
14:30:17 <openstack> Voted on "image-create-via-import should have sensible defaults?" Results are
14:30:23 <jokke_> #startvote image-create-via-import should have sensible defaults? Yes, No, users-choice
14:30:23 <openstack> Begin voting on: image-create-via-import should have sensible defaults? Valid vote options are Yes, No, users-choice.
14:30:25 <openstack> Vote using '#vote OPTION'. Only your last vote counts.
14:30:57 <smcginnis> #vote Yes
14:31:01 <McClymontS> #vote Yes
14:31:03 <jokke_> rosmaita: something that does not break the world and get similar which hunt on us as the 1.0.0 release did :P
14:31:14 <abhishekk> #vote Yes
14:31:17 <jokke_> #vote users-choice
14:31:40 <rosmaita> #vote users-choice
14:32:34 <jokke_> ok, do we have anyone else here who has opinion?
14:33:37 <smcginnis> User choice would be the environment variable right? The only thing I don't like about that is it's kind of a hidden thing that someone would need to go out and learn about.
14:34:20 <rosmaita> yes, we would have to publicise it
14:34:26 <jokke_> smcginnis: yes, although I do not think it's a bad idea even if we keep the default in place to have that to obey
14:34:27 <rosmaita> would show up in the help, i think
14:34:36 <McClymontS> From a use case perspective I like sensible defaults
14:35:04 <rosmaita> yes, it's like apple pie
14:35:15 <McClymontS> Who doesn't like apple pie
14:35:16 <rosmaita> not sure what the cross-cultural equivalent of that is
14:35:21 <jokke_> #endvote
14:35:22 <openstack> Voted on "image-create-via-import should have sensible defaults?" Results are
14:35:23 <openstack> users-choice (2): jokke_, rosmaita
14:35:25 <openstack> Yes (3): McClymontS, smcginnis, abhishekk
14:35:26 <smcginnis> If we can do user-choice, but have the no-env-variable-defined default be a reasonable one, then that's kind of the best of both voted options, right?
14:35:39 <rosmaita> smcginnis i think so
14:35:41 <smcginnis> User choice with reasonable default.
14:35:48 <smcginnis> I'm good with that then.
14:35:49 <jokke_> rosmaita: you've seen movie American Pie, right? :D
14:35:51 <McClymontS> yeah I think that meets in the middle
14:36:13 <abhishekk> :D
14:36:20 <rosmaita> ok, so can someone summarize what we agreed on? is the "sensible default" == glance-direct or None?
14:37:06 <jokke_> glance-direct would be the sensible backwards compatible default
14:37:13 <abhishekk> I guess sensible default
14:38:14 <rosmaita> ok, so i will put up a patch with glance-direct default and that recognizes the env var and revise the tests accordingly
14:38:22 <jokke_> unless you want to implement the magic into the client to detect what the user wants to do when they don't specify the method but provide payload ;)
14:38:43 <rosmaita> well, that's basically what we
14:38:50 <rosmaita> re doing
14:39:49 <rosmaita> but now there is no way to simply create an image record
14:39:53 <jokke_> umm nope ... we are telling that _if_ you do image create and provide your image file, we will create the image all the way like we used to, and we have options for you to choose other methods ;)
14:40:15 <rosmaita> because glance-direct requires data
14:40:28 <rosmaita> so this is also backward incompatible
14:40:45 <jokke_> rosmaita: yes there is, we just haven't implemented it in the current implementation. So we do fail if the payload is not there, while we should just create the record as we used to
14:41:23 <jokke_> and yes, I was thinking of that when I looked the code and I have bug text pretty much written for it ;)
14:41:51 <rosmaita> i am liking this less and less
14:42:19 <jokke_> ok so currently:
14:42:24 <rosmaita> so, the import-method will have to remain None
14:42:42 <rosmaita> but if you include --file or stdin we will know that you mean glance-direct and adjust
14:42:44 <jokke_> 1) image create without payload - creates image that stay's in queued
14:43:07 <rosmaita> we have to do it in the function, because the config-parser sets the default value
14:43:19 <jokke_> 2) image create with payload - creates image and uploads the payload == if everything goes fine active image
14:43:39 <jokke_> with the import version where we should be:
14:44:02 <jokke_> 1) image create without file - creates image that stays queues
14:44:48 <jokke_> 2) image create with payload file or stdin - creates image, stages the payload, imports == if everything goes fine active image
14:45:06 <jokke_> with import and any other method choosen:
14:45:39 <rosmaita> why don't we just drop the import-method altogether?
14:45:47 <jokke_> 1) provide all the needed parameters for that method - creates image, does what is needed by the method, imports == if everything goes fine active imagew
14:46:55 <McClymontS> I strongly dislike the image create to queued with no payload
14:47:46 <jokke_> McClymontS: there is a lot of legimite usecases to create image before providing the data and I'm not gonna let anyone break image-create around that
14:48:08 <McClymontS> Understood, I understand why it was built out
14:48:45 <McClymontS> Is it rate limited on default?
14:48:47 <McClymontS> I'm not even sure
14:49:17 <rosmaita> ok, so in the code, how do i distinguish between wanting to create an image record, and glance-direct where someone forgot the --file or stdin redirect?
14:49:33 <rosmaita> McClymontS glance does not do rate limiting, that has to be done externally
14:49:37 <jokke_> McClymontS: I hear what you say and I'm inclined to say that image-create should have never had the option to "magically" detect what the user might want, but I did not have a say to that decision
14:50:16 <McClymontS> Cool thanks for the context, rosmaita I'm not sure tbh
14:50:34 <jokke_> I think the best case scenario (and where rosmaita seems to want to move) would have been to tell the user, "This workflow needs to you run these commands and provide them their respective imput"
14:50:50 <jokke_> instead of "Dump everything to that first command and voodoo happens"
14:50:54 <rosmaita> you have detected my hidden agenda!
14:51:28 <jokke_> rosmaita: and that hidden agenda is backwards incompatibe  and I still remember there version 1.0.0 release way too well
14:51:57 <rosmaita> ok, so i still have my question above about how can i tell in the function
14:52:50 <rosmaita> what i mean is this: https://review.openstack.org/#/c/555550/2/glanceclient/v2/shell.py@105
14:52:54 <rosmaita> line 105
14:53:42 <rosmaita> if default='glance-direct', args.import_method will be == 'glance-direct' when control hits the function
14:54:28 <rosmaita> the problem is that this is a more complicated command than image-create was
14:55:05 <rosmaita> i mean, maybe the solution is just that we leave image-create as-is
14:55:09 <rosmaita> into the futurew
14:55:12 <jokke_> rosmaita: so your worry is that there is no way to tell if user explicitely called it with method glance-direct and forgot to provide the payload?
14:55:18 <rosmaita> exactly
14:55:25 <rosmaita> we want to minimize support calls
14:55:39 <rosmaita> i am thinking that this needs to be its own call
14:56:03 <rosmaita> its own command, i mean
14:56:50 <smcginnis> 4 minutes
14:56:57 <rosmaita> yeah, sorry about the time
14:56:59 <jokke_> ok, so I'm fone changing that defualt to None for that purpose and then we just need to specify that _if_ the method is not provided and payload is for backwards compatibility we default to glance-direct
14:57:08 <smcginnis> rosmaita: Meeting hog. :)
14:57:16 <jokke_> :%s/fone/fine/
14:57:29 <rosmaita> yeah, but the help text is going to be weird
14:57:37 <jokke_> and the we need to do that detection in the code, which is very trivial
14:57:41 <jokke_> I know
14:58:02 <jokke_> but it's way less weird if you write it rather than me
14:58:07 <rosmaita> :)
14:58:11 <jokke_> ok last 2min for open discussion
14:58:18 <jokke_> #topic open discussion
14:58:41 <rosmaita> yeah, one last thing ... if this is a new call, there are no backward-compat worries
14:58:53 <rosmaita> and operators are still going to want to use direct upload
14:59:06 <rosmaita> so i really wonder about this being a 1-1 replacement for image-create command
14:59:17 <jokke_> rosmaita: well it's not because we promised this as drop in replacement for users (IIRC the spec right)
14:59:31 <bhagyashris> jokke_: waiting for new upstream branch for policy refactor changes :)
14:59:43 <jokke_> bhagyashris: it's created forgot it from updates
14:59:44 <rosmaita> i don't remember the spec saying anything about client support
15:00:14 <bhagyashris> jokke_: ohh ok no problem
15:00:15 <jokke_> feature/policy-refactor
15:00:21 <jokke_> thanks all!
15:00:28 <jokke_> #endmeeting