| *** bhagyashris is now known as bhagyashris|ruck | 11:30 | |
| opendevreview | Erno Kuvaja proposed openstack/glance master: Sync example configs for Yoga https://review.opendev.org/c/openstack/glance/+/831368 | 12:41 |
|---|---|---|
| opendevreview | Merged openstack/glance master: Yoga RC-1 release notes https://review.opendev.org/c/openstack/glance/+/830834 | 13:12 |
| opendevreview | Pranali Deore proposed openstack/glance-tempest-plugin master: Implement API protection testing for metadef resource types https://review.opendev.org/c/openstack/glance-tempest-plugin/+/802792 | 13:36 |
| opendevreview | Pranali Deore proposed openstack/glance-tempest-plugin master: Implement API protection testing for metadef properties https://review.opendev.org/c/openstack/glance-tempest-plugin/+/802794 | 13:36 |
| opendevreview | Pranali Deore proposed openstack/glance-tempest-plugin master: Implement API protection testing for metadef tags https://review.opendev.org/c/openstack/glance-tempest-plugin/+/802795 | 13:36 |
| opendevreview | Brian Rosmaita proposed openstack/glance master: Change default value for [wsgi]/python_interpreter https://review.opendev.org/c/openstack/glance/+/831381 | 14:17 |
| rosmaita | dansmith: need your input on ^^ | 14:18 |
| dansmith | rosmaita: hmm, definitely seems bad to have to not have a sane default there just because of how we generate our config | 14:44 |
| dansmith | rosmaita: I think I'd rather use a static string of | 14:46 |
| dansmith | '/usr/bin/python3' than just None and override it anywhere we need to use it | 14:46 |
| dansmith | I know it should be minimal, but :/ | 14:46 |
| rosmaita | i guess i'm ok with the static string if we think there's a sensible one (which '/usr/bin/python3' seems to be) | 15:00 |
| rosmaita | dansmith: ^^ | 15:00 |
| dansmith | it's far from ideal, IMHO, but better than None | 15:01 |
| dansmith | I'd also like to ask gmann if he has any ideas, but he's in the board meeting and I'm trying to resolve something else.. can we circle back in a few hours? | 15:01 |
| rosmaita | dansmith: left a reply on the patch | 15:41 |
| rosmaita | and yeah, i'll be here all day | 15:42 |
| dansmith | is that the only place that gets used? | 15:43 |
| dansmith | it's been a while since i looked at it but I thought we had a couple places just in glance | 15:43 |
| dansmith | ...because I thought we had to set it for devstack | 15:44 |
| dansmith | it's set in .zuul, so if you're right we should be able to remove that and have the wsgi jobs still work | 15:45 |
| dansmith | ah, actually this is coming back to me now | 15:47 |
| dansmith | in any real wsgi environment, the sys.executable will be something like /usr/bin/uwsgi, which is why taking the default doesn't actually work | 15:48 |
| dansmith | and why we have to override it in our own jobs | 15:48 |
| dansmith | so maybe I was just conflating that with it not being right at runtime for normal stuff either | 15:48 |
| dansmith | so I guess if None triggers the behavior we had before I added that for not-real-wsgi environments, then that's fine | 15:49 |
| dansmith | rosmaita: replied, sorry for the distraction | 15:53 |
| rosmaita | np, i think that's right (None triggers the behavior we had before), but i definitely needed some eyes on this | 15:55 |
| dansmith | there's coverage both directions on this in zuul, so the test report will help confirm all that | 15:57 |
| rosmaita | cool | 15:58 |
| opendevreview | Erno Kuvaja proposed openstack/glance master: Sync example configs for Yoga https://review.opendev.org/c/openstack/glance/+/831368 | 16:14 |
| dansmith | rosmaita: I think your recheck of that thing missed this: https://zuul.opendev.org/t/openstack/build/9c4fbdb6743c4af4a877f56dddd9696e/log/controller/logs/screen-g-api.txt#514 | 16:25 |
| dansmith | complaining about None being passed to processutils.execute, which is your change | 16:26 |
| rosmaita | well, that sucks | 16:26 |
| dansmith | rosmaita: passing None there doesn't trigger that defaulting behavior, | 16:27 |
| dansmith | because we pop None from the kwargs, which means it never takes the default arg from the .get() | 16:27 |
| dansmith | if you're okay with it, I'll fix it up and will write a test | 16:31 |
| rosmaita | i am certainly ok with that | 16:34 |
| opendevreview | Pranali Deore proposed openstack/glance-tempest-plugin master: Implement API protection testing for metadef resource types https://review.opendev.org/c/openstack/glance-tempest-plugin/+/802792 | 16:37 |
| opendevreview | Pranali Deore proposed openstack/glance-tempest-plugin master: Implement API protection testing for metadef properties https://review.opendev.org/c/openstack/glance-tempest-plugin/+/802794 | 16:37 |
| opendevreview | Pranali Deore proposed openstack/glance-tempest-plugin master: Implement API protection testing for metadef tags https://review.opendev.org/c/openstack/glance-tempest-plugin/+/802795 | 16:37 |
| opendevreview | Dan Smith proposed openstack/glance master: Change default value for [wsgi]/python_interpreter https://review.opendev.org/c/openstack/glance/+/831381 | 16:37 |
| dansmith | rosmaita: ^ | 16:37 |
| dansmith | don't love it, but alas | 16:37 |
| *** bhagyashris_ is now known as bhagyashris|ruck | 16:43 | |
| rosmaita | yeah, i think processutils should have "python_exec = kwargs.pop('python_exec', None) or sys.executable" ... because what' the use case for python_exec to be None inside the execute function? | 16:45 |
| dansmith | yeah it'd be ideal if it did, but that's a whole thing | 16:45 |
| rosmaita | right, in the absence of that, i think your change is the best we can do | 16:46 |
| dansmith | yeah | 16:46 |
| rosmaita | i'll file a oslo bug, because that's pretty counterintuitive | 16:46 |
| dansmith | I really thought we had to use this in more than one place in glance, but it clearly isn't the case at the moment | 16:46 |
| dansmith | so I must be remembering something other than the final situation | 16:47 |
| dansmith | rosmaita: ++ | 16:47 |
| gmann | dansmith: rosmaita just read the logs and comment in review. static string might go wrong in few place if people does not notice the default change. I agree on None as best way. | 16:55 |
| gmann | +1 on fixing oslo on that | 16:55 |
| gmann | rosmaita: dansmith and good to add release notes for default change? | 16:55 |
| dansmith | gmann: is there any option to override how the default is generated in the config generator or something? | 16:55 |
| rosmaita | gmann: ok, i will file a bug and add a patch for oslo.concurrency | 16:56 |
| dansmith | because it would be a lot better if we could keep the conf default correct, IMHO | 16:56 |
| dansmith | I'm not sure we need a reno because the default hasn't actually changed, just where we apply the default | 16:56 |
| dansmith | but obviously renos are cheap, if others think it's important | 16:57 |
| gmann | dansmith: afaik, if we override the default via set_defaults then only config generator take that. but that will be not just for config generator but for all code base | 16:58 |
| dansmith | well, I just hope that if we have to add any other cases of execute() we'll refactor this into a helper (and remember to use that instead of the actual conf value) | 16:59 |
| rosmaita | i think a reno might be more confusing than helpful | 16:59 |
| dansmith | anyone writing a plugin that uses execute() will have to do the same | 16:59 |
| gmann | dansmith: one way is explicitly setting here which can control it in glance side https://github.com/openstack/glance/blob/a693e066b3a713e0cc76389837593cde472cb9c8/glance/common/config.py#L758 | 17:04 |
| gmann | https://docs.openstack.org/oslo.config/latest/cli/generator.html#modifying-defaults-from-other-namespaces | 17:05 |
| dansmith | that's kinda manipulating the defaults at runtime though right? | 17:05 |
| gmann | yeah | 17:05 |
| dansmith | yeah, that's pretty :( | 17:06 |
| dansmith | I'd rather something that modified the generator code than the runtime code, so if that's not a thing, then what we're doing is probably better | 17:06 |
| dansmith | I just wonder if the generator could be augmented to allow for some overrides only during generation, | 17:06 |
| dansmith | like "wsgi.interpreter should be overridden with this string" | 17:07 |
| dansmith | but this might be the only use-case, so not worth a whole big dust-up just for this | 17:07 |
| dansmith | I know that also kinda defeats the point of defaults in code, but.. the default in code at runtime seems better than the default in code for the generator | 17:07 |
| dansmith | I dunno | 17:07 |
| gmann | yeah, its both way. but if runtime is always taken as priority then override during generation can also be supported. but it may lead to error when both are done without knowing it :) | 17:10 |
| gmann | same as legacy quotas :) many of us still struggle to understand which one take priority and what the last I set :). thanks to unified limit :) | 17:12 |
| dansmith | heh | 17:18 |
| opendevreview | Ghanshyam proposed openstack/glance master: Add grenade-skip-level irrelevant-files config https://review.opendev.org/c/openstack/glance/+/831437 | 19:09 |
| gmann | dansmith: ^^ | 19:11 |
| * dansmith nods | 19:12 | |
| *** dtantsur_ is now known as dtantsur | 20:24 | |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!