| kragniz | thanks for the help | 00:00 |
|---|---|---|
| harlowja_ | np | 00:00 |
| *** jaosorior has quit IRC | 00:01 | |
| openstackgerrit | Merged openstack/oslo.config: Fixes deprecation warning for oslo.config in cfg.py https://review.openstack.org/156730 | 00:06 |
| openstackgerrit | Merged openstack/oslo.config: Support i18n messages in config generator https://review.openstack.org/145280 | 00:15 |
| openstackgerrit | Merged openstack/oslo.log: Update Oslo imports to remove namespace package https://review.openstack.org/149090 | 00:26 |
| *** jaypipes has quit IRC | 00:27 | |
| himangi | dhellmann: I have got myself setup | 00:30 |
| *** dims__ has quit IRC | 00:33 | |
| *** dims__ has joined #openstack-oslo | 00:33 | |
| *** dims__ has quit IRC | 00:33 | |
| *** dims__ has joined #openstack-oslo | 00:35 | |
| openstackgerrit | Merged openstack/oslo.config: Add a list_all_sections method to ConfigOpts https://review.openstack.org/148936 | 00:37 |
| himangi | which is the development repository for oslo? | 00:37 |
| *** bknudson has quit IRC | 00:41 | |
| *** david-lyle has quit IRC | 00:44 | |
| *** hemna is now known as hemnafk | 01:08 | |
| *** achanda has quit IRC | 01:17 | |
| *** zzzeek has quit IRC | 01:22 | |
| *** zzzeek has joined #openstack-oslo | 01:31 | |
| *** sigmavirus24 is now known as sigmavirus24_awa | 01:38 | |
| *** salv-orlando has quit IRC | 01:58 | |
| *** stevemar has quit IRC | 02:09 | |
| *** david-lyle has joined #openstack-oslo | 02:11 | |
| *** mriedem has joined #openstack-oslo | 02:14 | |
| *** mriedem has quit IRC | 02:15 | |
| *** mriedem has joined #openstack-oslo | 02:16 | |
| openstackgerrit | Joshua Harlow proposed openstack/taskflow: WIP for having the ability to have tooz find workers+topics https://review.openstack.org/151495 | 02:19 |
| *** stevemar has joined #openstack-oslo | 02:22 | |
| *** takedakn has joined #openstack-oslo | 02:24 | |
| *** takedakn has quit IRC | 02:26 | |
| *** noelbk has quit IRC | 02:26 | |
| *** takedakn has joined #openstack-oslo | 02:26 | |
| *** tsekiyam_ has joined #openstack-oslo | 02:33 | |
| *** tsekiyama has quit IRC | 02:37 | |
| *** tsekiyam_ has quit IRC | 02:38 | |
| *** mtanino has quit IRC | 02:43 | |
| *** takedakn has quit IRC | 02:48 | |
| *** salv-orlando has joined #openstack-oslo | 02:59 | |
| openstackgerrit | Merged openstack-dev/hacking: H105: also check for Authors and authors https://review.openstack.org/147201 | 03:03 |
| openstackgerrit | Merged openstack/oslo-incubator: Link hacking guidelines into dev docs https://review.openstack.org/152238 | 03:07 |
| openstackgerrit | Merged openstack/oslo-incubator: Have a little fun with release notes https://review.openstack.org/152680 | 03:08 |
| *** boris-42 has quit IRC | 03:12 | |
| *** yamahata has quit IRC | 03:13 | |
| *** himangi has quit IRC | 03:21 | |
| *** harlowja_ is now known as harlowja_away | 03:22 | |
| *** rushiagr_away is now known as rushiagr | 03:28 | |
| *** david-lyle has quit IRC | 03:35 | |
| *** david-lyle has joined #openstack-oslo | 03:35 | |
| *** david-lyle has quit IRC | 03:40 | |
| *** mriedem has left #openstack-oslo | 03:42 | |
| *** mriedem has quit IRC | 03:42 | |
| *** tsekiyama has joined #openstack-oslo | 03:44 | |
| openstackgerrit | Merged openstack/oslo.config: Do not import our namespace package https://review.openstack.org/156820 | 03:48 |
| *** tsekiyama has quit IRC | 03:48 | |
| *** dims__ has quit IRC | 03:49 | |
| *** harlowja_at_home has joined #openstack-oslo | 03:50 | |
| *** salv-orlando has quit IRC | 03:51 | |
| *** amotoki has joined #openstack-oslo | 03:54 | |
| openstackgerrit | Joshua Harlow proposed openstack/taskflow: Tweaks to atom documentation https://review.openstack.org/156844 | 04:16 |
| *** takedakn has joined #openstack-oslo | 04:19 | |
| *** stevemar has quit IRC | 04:20 | |
| *** harlowja_at_home has quit IRC | 04:21 | |
| *** david-lyle has joined #openstack-oslo | 04:22 | |
| *** david-lyle is now known as david-lyle_afk | 04:23 | |
| *** achanda has joined #openstack-oslo | 04:29 | |
| *** takedakn has quit IRC | 04:33 | |
| *** stevemar has joined #openstack-oslo | 04:36 | |
| *** achanda has quit IRC | 04:45 | |
| openstackgerrit | Merged openstack/taskflow: Add todo note for kombu pull request https://review.openstack.org/156472 | 04:46 |
| *** dims__ has joined #openstack-oslo | 04:50 | |
| openstackgerrit | Eric Brown proposed openstack/oslo.vmware: Add bandit to tox for security static analysis https://review.openstack.org/156838 | 04:52 |
| *** achanda has joined #openstack-oslo | 04:52 | |
| *** dims__ has quit IRC | 04:54 | |
| *** achanda has quit IRC | 04:59 | |
| *** achanda has joined #openstack-oslo | 05:00 | |
| openstackgerrit | Merged openstack/taskflow: Improve multilock class and its associated unit test https://review.openstack.org/155663 | 05:02 |
| *** achanda has quit IRC | 05:11 | |
| *** yamahata has joined #openstack-oslo | 05:33 | |
| *** salv-orlando has joined #openstack-oslo | 05:36 | |
| *** zzzeek has quit IRC | 05:43 | |
| openstackgerrit | Eric Brown proposed openstack/oslo.vmware: Resolve warnings from Bandit https://review.openstack.org/156889 | 05:50 |
| *** achanda has joined #openstack-oslo | 05:53 | |
| openstackgerrit | OpenStack Proposal Bot proposed openstack/oslo.db: Imported Translations from Transifex https://review.openstack.org/156890 | 06:01 |
| openstackgerrit | Joshua Harlow proposed openstack/taskflow: Use the enum library for the retry strategy enumerations https://review.openstack.org/156893 | 06:02 |
| *** crc32 has quit IRC | 06:03 | |
| openstackgerrit | OpenStack Proposal Bot proposed openstack/oslo.vmware: Imported Translations from Transifex https://review.openstack.org/156897 | 06:04 |
| *** ajo_ has joined #openstack-oslo | 06:08 | |
| openstackgerrit | Joshua Harlow proposed openstack/taskflow: Use the enum library for the retry strategy enumerations https://review.openstack.org/156893 | 06:09 |
| *** ajo has quit IRC | 06:11 | |
| openstackgerrit | Joshua Harlow proposed openstack/taskflow: Use the enum library for the retry strategy enumerations https://review.openstack.org/156893 | 06:14 |
| *** achanda has quit IRC | 06:20 | |
| *** achanda has joined #openstack-oslo | 06:22 | |
| *** exploreshaifali has joined #openstack-oslo | 06:24 | |
| *** achanda has quit IRC | 06:32 | |
| *** exploreshaifali has quit IRC | 06:33 | |
| *** ajo_ is now known as ajo | 06:35 | |
| *** miqui has joined #openstack-oslo | 06:41 | |
| *** achanda has joined #openstack-oslo | 06:56 | |
| *** takedakn has joined #openstack-oslo | 07:10 | |
| *** exploreshaifali has joined #openstack-oslo | 07:18 | |
| *** vigneshvar has joined #openstack-oslo | 07:31 | |
| *** miqui has quit IRC | 07:39 | |
| *** miqui has joined #openstack-oslo | 07:41 | |
| *** takedakn has quit IRC | 07:48 | |
| *** miqui has quit IRC | 07:49 | |
| *** salv-orlando has quit IRC | 07:51 | |
| *** andreykurilin_ has joined #openstack-oslo | 07:53 | |
| *** takedakn has joined #openstack-oslo | 07:58 | |
| *** boris-42 has joined #openstack-oslo | 08:00 | |
| *** miqui has joined #openstack-oslo | 08:06 | |
| *** takedakn has quit IRC | 08:07 | |
| *** takedakn has joined #openstack-oslo | 08:15 | |
| *** yamahata has quit IRC | 08:15 | |
| *** yamahata has joined #openstack-oslo | 08:16 | |
| *** ajo has quit IRC | 08:18 | |
| *** rushiagr is now known as rushiagr_away | 08:21 | |
| *** viktors|afk is now known as viktors | 08:27 | |
| *** rushiagr_away is now known as rushiagr | 08:28 | |
| *** miqui has quit IRC | 08:30 | |
| *** stevemar has quit IRC | 08:31 | |
| *** achanda has quit IRC | 08:31 | |
| *** inc0 has joined #openstack-oslo | 08:33 | |
| *** himangi has joined #openstack-oslo | 08:34 | |
| *** ajo has joined #openstack-oslo | 08:35 | |
| *** salv-orlando has joined #openstack-oslo | 08:48 | |
| *** andreykurilin_ has quit IRC | 08:54 | |
| *** ajo has quit IRC | 08:56 | |
| *** salv-orlando has quit IRC | 09:01 | |
| *** ajo has joined #openstack-oslo | 09:01 | |
| *** salv-orlando has joined #openstack-oslo | 09:01 | |
| *** i159 has joined #openstack-oslo | 09:04 | |
| *** salv-orlando has quit IRC | 09:10 | |
| *** salv-orlando has joined #openstack-oslo | 09:10 | |
| *** exploreshaifali has quit IRC | 09:17 | |
| *** dulek has joined #openstack-oslo | 09:18 | |
| openstackgerrit | Michal Jastrzebski (inc0) proposed openstack/oslo.versionedobjects: Fixes for heat implementation https://review.openstack.org/154835 | 09:30 |
| *** takedakn has quit IRC | 09:30 | |
| *** salv-orlando has quit IRC | 09:32 | |
| *** salv-orlando has joined #openstack-oslo | 09:32 | |
| *** salv-orlando has quit IRC | 09:35 | |
| *** salv-orlando has joined #openstack-oslo | 09:36 | |
| *** jecarey_ has quit IRC | 09:37 | |
| *** salv-orlando has quit IRC | 09:45 | |
| *** salv-orlando has joined #openstack-oslo | 09:46 | |
| *** ihrachyshka has joined #openstack-oslo | 09:49 | |
| *** salv-orl_ has joined #openstack-oslo | 09:49 | |
| *** salv-orl_ has quit IRC | 09:51 | |
| *** salv-orlando has quit IRC | 09:51 | |
| *** salv-orlando has joined #openstack-oslo | 09:51 | |
| openstackgerrit | Marian Horban proposed openstack/oslo-incubator: Using of waitpid system call was optimized https://review.openstack.org/156345 | 09:53 |
| *** salv-orlando has quit IRC | 09:56 | |
| *** salv-orlando has joined #openstack-oslo | 09:56 | |
| *** exploreshaifali has joined #openstack-oslo | 10:03 | |
| *** yamahata has quit IRC | 10:15 | |
| *** takedakn has joined #openstack-oslo | 10:24 | |
| *** takedakn has quit IRC | 10:32 | |
| *** takedakn has joined #openstack-oslo | 10:34 | |
| *** takedakn has quit IRC | 10:56 | |
| *** kbyrne has quit IRC | 11:09 | |
| *** exploreshaifali has quit IRC | 11:11 | |
| *** kbyrne has joined #openstack-oslo | 11:13 | |
| *** cdent has joined #openstack-oslo | 11:13 | |
| *** e0ne has joined #openstack-oslo | 11:14 | |
| *** dims__ has joined #openstack-oslo | 11:19 | |
| *** amotoki has quit IRC | 11:20 | |
| *** exploreshaifali has joined #openstack-oslo | 11:24 | |
| *** exploreshaifali has quit IRC | 11:28 | |
| *** e0ne is now known as e0ne_ | 11:40 | |
| dims__ | ihrachyshka: "Reach out, fix, help. Not complain." - golden words! | 11:42 |
| *** e0ne_ has quit IRC | 11:50 | |
| *** takedakn has joined #openstack-oslo | 11:53 | |
| *** e0ne has joined #openstack-oslo | 12:18 | |
| *** salv-orlando has quit IRC | 12:20 | |
| *** takedakn has quit IRC | 12:24 | |
| *** amotoki has joined #openstack-oslo | 12:37 | |
| *** e0ne is now known as e0ne_ | 13:05 | |
| *** jaypipes has joined #openstack-oslo | 13:05 | |
| *** himangi has joined #openstack-oslo | 13:09 | |
| *** e0ne_ is now known as e0ne | 13:09 | |
| *** rushiagr is now known as rushiagr_away | 13:09 | |
| *** kgiusti has joined #openstack-oslo | 13:15 | |
| *** zigo has quit IRC | 13:18 | |
| *** zigo has joined #openstack-oslo | 13:20 | |
| *** e0ne_ has joined #openstack-oslo | 13:24 | |
| *** salv-orlando has joined #openstack-oslo | 13:25 | |
| *** e0ne has quit IRC | 13:27 | |
| *** e0ne has joined #openstack-oslo | 13:30 | |
| *** e0ne_ has quit IRC | 13:33 | |
| *** gordc has joined #openstack-oslo | 13:37 | |
| *** trown has quit IRC | 13:49 | |
| *** vigneshvar has quit IRC | 13:49 | |
| *** himangi has quit IRC | 13:50 | |
| *** trown has joined #openstack-oslo | 13:50 | |
| *** jecarey has joined #openstack-oslo | 13:51 | |
| *** rluediger has joined #openstack-oslo | 13:59 | |
| *** rluediger has quit IRC | 14:01 | |
| *** jecarey has quit IRC | 14:03 | |
| *** exploreshaifali has joined #openstack-oslo | 14:04 | |
| *** rushiagr_away is now known as rushiagr | 14:07 | |
| *** dims__ has quit IRC | 14:15 | |
| *** dims__ has joined #openstack-oslo | 14:20 | |
| *** dims__ has quit IRC | 14:20 | |
| *** dims__ has joined #openstack-oslo | 14:21 | |
| *** dims__ has quit IRC | 14:25 | |
| *** amrith is now known as _amrith_ | 14:29 | |
| *** mriedem has joined #openstack-oslo | 14:31 | |
| *** Guest37356 has joined #openstack-oslo | 14:35 | |
| *** jgrimm is now known as zz_jgrimm | 14:38 | |
| *** Guest37356 is now known as dims__ | 14:49 | |
| *** ajo has quit IRC | 14:51 | |
| *** ajo has joined #openstack-oslo | 14:51 | |
| dansmith | inc0: replied to your comments | 14:52 |
| inc0 | yeah, I saw it, removing -1 :) | 14:53 |
| dansmith | heh, okay | 14:53 |
| inc0 | also, I've added this patch as requirements to my | 14:53 |
| inc0 | mine* | 14:53 |
| dansmith | inc0: okay, on the timezone thing, what you have looks fine, but since it's part of the api: | 14:54 |
| * dansmith thinks | 14:55 | |
| dansmith | what you have here lets me store a tz-aware or a tz-unaware datetime in the field if ensure_tzinfo=False | 14:56 |
| dansmith | which means that you could get two Foo objects, one with an aware datetime and one without, and then be unable to compare them | 14:56 |
| inc0 | yeah, basically what it do is "what you put in, you'll get out" | 14:56 |
| dims__ | dansmith: there was a question from harlowja_away in https://review.openstack.org/#/c/156398/ | 14:57 |
| dansmith | I wonder if it would be better to have the flag be "is_tz_aware", so that you either have tz-aware datetimes, forced to utc if they're unaware in the positive case, and tzinfo is stripped out if negative | 14:57 |
| inc0 | I don't see point of removing tzinfo if its there | 14:57 |
| dansmith | it's just to try to ensure consistency of what you have stored, since you can't compare aware and non-aware datetimes | 14:58 |
| inc0 | I don't think its good eighter...its deleting data from object | 14:58 |
| dansmith | well, you could just raise an exception instead of stripping it | 14:58 |
| dansmith | what I meant is, ensure that the awareness matches the flag | 14:59 |
| inc0 | that would be better | 14:59 |
| inc0 | allright, I'll do that | 14:59 |
| *** salv-orlando has quit IRC | 15:00 | |
| dansmith | inc0: okay, after that, I think we should merge that and I promise to stop arguing about it :) | 15:00 |
| dims__ | inc0: is your primary focus using versionedobjects in heat? (any other project?) | 15:00 |
| inc0 | dansmith, sounds good | 15:01 |
| inc0 | dims__, I've started from heat | 15:01 |
| inc0 | but now I'm helping guys from cinder with implementation | 15:01 |
| inc0 | and other to get specs for L going;) like glance, neutron | 15:01 |
| *** salv-orlando has joined #openstack-oslo | 15:02 | |
| dims__ | inc0: nice | 15:03 |
| *** jecarey_ has joined #openstack-oslo | 15:03 | |
| *** mtanino has joined #openstack-oslo | 15:03 | |
| *** daniel3_ has joined #openstack-oslo | 15:04 | |
| dims__ | inc0: waiting for a few more reviews from you on the series - https://review.openstack.org/#/c/156662/ | 15:05 |
| dims__ | when you get a chance | 15:05 |
| inc0 | I did like half of them before my manager stormed in and called a meeting;) I'll get back to it as soon as I fix my patch | 15:06 |
| dims__ | haha, thanks | 15:06 |
| dims__ | inc0: this one right? https://review.openstack.org/#/c/154835/ | 15:06 |
| inc0 | yup | 15:07 |
| *** cdent_ has joined #openstack-oslo | 15:09 | |
| *** cdent has quit IRC | 15:09 | |
| *** cdent_ is now known as cdent | 15:09 | |
| inc0 | dansmith, on the closer look, I'll go for removing tzinfo, because timeutils injects it anyway;) | 15:15 |
| inc0 | so by this flag I'll just ensure both ends are eighter tzinfo aware, or not, wile inside it will always be tzinfo aware | 15:15 |
| *** sigmavirus24_awa is now known as sigmavirus24 | 15:16 | |
| *** _amrith_ is now known as amrith | 15:18 | |
| *** e0ne is now known as e0ne_ | 15:18 | |
| *** e0ne_ is now known as e0ne | 15:20 | |
| *** zz_jgrimm is now known as jgrimm | 15:22 | |
| *** himangi has joined #openstack-oslo | 15:24 | |
| *** stevemar has joined #openstack-oslo | 15:24 | |
| openstackgerrit | Michal Jastrzebski (inc0) proposed openstack/oslo.versionedobjects: Fixes for heat implementation https://review.openstack.org/154835 | 15:27 |
| dansmith | inc0: heh, okay, will look in a sec | 15:27 |
| dhellmann | good morning, all | 15:27 |
| inc0 | thanks | 15:30 |
| inc0 | hello dhellmann | 15:30 |
| dhellmann | dims__: do you have some time this morning for some policy reviews? I have a few cleanups queued up and if we can get them in today I'd like them to be in the first release | 15:30 |
| dhellmann | hi, inc0 | 15:30 |
| dhellmann | dims__: that's oslo.policy, sorry, wasn't clear | 15:31 |
| dims__ | dhellmann: in an hour, yes. will do | 15:31 |
| dhellmann | dims__: great, thanks! | 15:31 |
| *** devlaps has joined #openstack-oslo | 15:32 | |
| *** exploreshaifali has quit IRC | 15:32 | |
| inc0 | dims__, https://review.openstack.org/#/c/156397/ I think we should explicitly say that its for tests | 15:34 |
| inc0 | hmf...maybe just file naming? | 15:35 |
| inc0 | like test_utils.py or fakes.py? what do you think? | 15:37 |
| openstackgerrit | Merged openstack/oslo.versionedobjects: Disable some unstable tests until they are generalized https://review.openstack.org/156660 | 15:37 |
| dims__ | inc0: we have used fixture.py in other oslo projects | 15:43 |
| dims__ | dims@dims-mac:~/openstack/oslo$ find . -name fixture.py | 15:43 |
| dims__ | ./oslo.config/oslo/config/fixture.py | 15:43 |
| dims__ | ./oslo.config/oslo_config/fixture.py | 15:43 |
| dims__ | ./oslo.context/oslo_context/fixture.py | 15:43 |
| dims__ | ./oslo.i18n/oslo/i18n/fixture.py | 15:43 |
| dims__ | ./oslo.i18n/oslo_i18n/fixture.py | 15:43 |
| dims__ | ./oslo.utils/oslo_utils/fixture.py | 15:43 |
| *** zzzeek has joined #openstack-oslo | 15:43 | |
| inc0 | than maybe stick to this name? | 15:43 |
| dims__ | but i am flexible, dont want to mandate | 15:43 |
| inc0 | well, I'm ok with fixture, especially if its present in rest of oslo | 15:44 |
| inc0 | my point is, testing alltogether has its own nomenclature, and I'd like to stick to it when we produce something test related | 15:44 |
| inc0 | (but I'm not stubborn about it) | 15:45 |
| openstackgerrit | Merged openstack/oslo.versionedobjects: Generalize the indirection_api interface https://review.openstack.org/156394 | 15:46 |
| openstackgerrit | Merged openstack/oslo.versionedobjects: Remove Nova objects module registration code https://review.openstack.org/156395 | 15:46 |
| openstackgerrit | Merged openstack/oslo.versionedobjects: Add conditional object registration https://review.openstack.org/156396 | 15:46 |
| openstackgerrit | Merged openstack/oslo.versionedobjects: Generalize remote testing infrastructure https://review.openstack.org/156397 | 15:46 |
| dims__ | inc0: i hate to suggest renaming stuff in the middle of a patch series as well :) | 15:47 |
| inc0 | yeah, we can do it as another patch, works for me | 15:47 |
| inc0 | this is just my idea of possible improvement, not mistake by itself | 15:48 |
| *** hemnafk is now known as hemna | 15:49 | |
| *** jecarey_ is now known as jecarey | 15:49 | |
| *** jaosorior has joined #openstack-oslo | 15:53 | |
| *** yamahata has joined #openstack-oslo | 15:58 | |
| *** viktors is now known as viktors|afk | 16:02 | |
| openstackgerrit | Doug Hellmann proposed openstack/oslo.policy: Clean up configuration option management https://review.openstack.org/157044 | 16:02 |
| inc0 | dims__, would you do the honors?:) https://review.openstack.org/#/c/154835/ | 16:06 |
| *** belliott has left #openstack-oslo | 16:08 | |
| *** inc0 has quit IRC | 16:09 | |
| *** harlowja_at_home has joined #openstack-oslo | 16:14 | |
| openstackgerrit | Steve Martinelli proposed openstack/oslo-incubator: Remove policy from oslo-incubator https://review.openstack.org/152812 | 16:16 |
| openstackgerrit | Steve Martinelli proposed openstack/oslo-incubator: Remove policy from oslo-incubator https://review.openstack.org/152812 | 16:16 |
| openstackgerrit | Steve Martinelli proposed openstack/oslo-incubator: Prevent update.py from updating policy https://review.openstack.org/152813 | 16:17 |
| *** tsekiyama has joined #openstack-oslo | 16:18 | |
| *** pradk has joined #openstack-oslo | 16:24 | |
| stevemar | dims__, thanksfor reviewing | 16:27 |
| *** david-lyle_afk is now known as david-lyle | 16:28 | |
| dims__ | stevemar: waiting for one more review (jobs still running) | 16:34 |
| *** harlowja_at_home has quit IRC | 16:43 | |
| haypo | do you know if eventlet supports reentrant lock? | 16:43 |
| *** noelbk has joined #openstack-oslo | 16:46 | |
| ihrachyshka | haypo, no idea about eventlet specifically, but for lockutils, it seems all our locking mechanisms are not aware of the same threads locking semaphores. so double lock locks the thread | 16:46 |
| haypo | ihrachyshka: it's writing an article about dead locks in threads, asyncio and eventlet ;) | 16:47 |
| openstackgerrit | Merged openstack/oslo.versionedobjects: Generalize object hash-based change detection https://review.openstack.org/156398 | 16:53 |
| openstackgerrit | Merged openstack/oslo.versionedobjects: Generalize object dependency change detection https://review.openstack.org/156399 | 16:53 |
| openstackgerrit | Merged openstack/oslo.versionedobjects: Add a test to ensure subclassibility of the object registry https://review.openstack.org/156662 | 16:58 |
| openstackgerrit | Merged openstack/oslo.policy: Create the temporary files needed for tests https://review.openstack.org/156811 | 17:03 |
| openstackgerrit | Merged openstack/oslo.policy: Change default set of tox environments https://review.openstack.org/156812 | 17:03 |
| openstackgerrit | Merged openstack/oslo.versionedobjects: Fixes for heat implementation https://review.openstack.org/154835 | 17:09 |
| openstackgerrit | Merged openstack/oslo.policy: Fix i18n imports https://review.openstack.org/156813 | 17:10 |
| *** amotoki has quit IRC | 17:10 | |
| openstackgerrit | Merged openstack/oslo.policy: Update comments about tox configuration https://review.openstack.org/156836 | 17:10 |
| *** i159 has quit IRC | 17:11 | |
| dansmith | dims__: hey, you merged all my patches! | 17:15 |
| *** daniel3_ has quit IRC | 17:17 | |
| *** daniel3_ has joined #openstack-oslo | 17:18 | |
| *** ihrachyshka has quit IRC | 17:18 | |
| *** daniel3_ has quit IRC | 17:18 | |
| *** daniel3_ has joined #openstack-oslo | 17:18 | |
| *** e0ne is now known as e0ne_ | 17:22 | |
| *** e0ne_ is now known as e0ne | 17:24 | |
| openstackgerrit | Ben Nemec proposed openstack/oslo.config: Log a warning when deprecated opts are used https://review.openstack.org/148020 | 17:27 |
| *** e0ne has quit IRC | 17:31 | |
| *** dulek has quit IRC | 17:32 | |
| *** bknudson has joined #openstack-oslo | 17:33 | |
| *** dulek has joined #openstack-oslo | 17:33 | |
| dims__ | dansmith: is that good or bad? :) | 17:34 |
| dansmith | dims__: good :) | 17:34 |
| dhellmann | bnemec: someone made a comment yesterday that https://review.openstack.org/#/c/154642/1/specs/eventlet-best-practices.rst,cm seemed like a poor fit for the specs repo, because it's not really proposing a 'change' per se. I wonder if it fits better in the oslo.concurrency docs? | 17:34 |
| dims__ | dansmith: mind if i propose a rename of checks.py -> fixture.py | 17:35 |
| bnemec | dhellmann: I would be fine with it living there, but I think it needs cross-project buy-in. | 17:35 |
| dansmith | dims__: we have a fixtures thing already, maybe they should be merged | 17:36 |
| dims__ | dansmith: ok, i'll take a stab | 17:36 |
| bnemec | dhellmann: Also, it's not really about how people use oslo.concurrency. Those rules would apply regardless of whether you're using Oslo at all. | 17:36 |
| dansmith | dims__: I'm proposing more changes to checks, so I'll slap a merge after I'm done | 17:36 |
| dhellmann | bnemec: yeah, that makes sense, I just wanted to mention it before I forgot | 17:36 |
| dansmith | dims__: or I can let you do it | 17:36 |
| dhellmann | bnemec: yeah, we don't have a good place for docs like this | 17:36 |
| dims__ | dansmith: cool, not in a hurry | 17:36 |
| *** daniel3_ has quit IRC | 17:36 | |
| dansmith | dims__: okay | 17:36 |
| *** daniel3_ has joined #openstack-oslo | 17:37 | |
| *** harlowja_away is now known as harlowja_ | 17:42 | |
| *** exploreshaifali has joined #openstack-oslo | 17:43 | |
| openstackgerrit | Doug Hellmann proposed openstack/oslo.policy: Clean up configuration option management https://review.openstack.org/157044 | 17:47 |
| harlowja_ | haypo afaik ya, it should work fine with reentrant locks (at least in 2.7/2.6) ; https://hg.python.org/releasing/2.7.9/file/753a8f457ddc/Lib/threading.py#l168 is replaced by eventlet (_get_ident() there) | 17:57 |
| harlowja_ | whether it works in py3.x i'm not sure | 17:57 |
| harlowja_ | * https://github.com/eventlet/eventlet/blob/master/eventlet/green/threading.py#L11 | 17:58 |
| harlowja_ | so that prevents the eventlet sempahore (which replaces the threading lock) from being acquired twice by the same greenthread | 17:58 |
| dhellmann | bnemec: looking at https://review.openstack.org/#/c/148020 -- I wondered why you didn't use versionutils to emit the warning? | 18:07 |
| *** dulek has quit IRC | 18:09 | |
| *** rushiagr is now known as rushiagr_away | 18:10 | |
| bnemec | dhellmann: I don't think I realized there was anything but the decorator in there. That should eliminate the duplication logic though. | 18:12 |
| *** yamahata has quit IRC | 18:12 | |
| dhellmann | bnemec: yeah, it also gives us the "die on a deprecated option" behavior that some operators like | 18:12 |
| dhellmann | bnemec: comment left -- I just wanted to make sure there wasn't a circular dependency reason or something | 18:13 |
| bnemec | dhellmann: Oh, also we have no incubator code in oslo.config today. | 18:13 |
| *** palendae has left #openstack-oslo | 18:13 | |
| dhellmann | that's not an issue by itself | 18:14 |
| bnemec | I guess we have the cross-test script. | 18:14 |
| bnemec | Yeah, I'll add versionutils. | 18:14 |
| dhellmann | there might be a circular dependency issue, though, since that module uses oslo_config | 18:14 |
| dhellmann | well, hang on | 18:14 |
| dhellmann | bnemec: if versionutils is going to end up in a library other than oslo.config, we'd have an issue with dependencies :-/ | 18:15 |
| * dhellmann needs lunch before puzzling this one out | 18:16 | |
| bnemec | Oh, update.py isn't aware of the namespace change yet. It's still putting files in oslo.config and hosing up the imports. :-( | 18:25 |
| bnemec | Ah, not update.py. Just openstack-common.conf. That's an easier fix. :-) | 18:26 |
| openstackgerrit | Merged openstack/oslo.db: Imported Translations from Transifex https://review.openstack.org/156890 | 18:29 |
| openstackgerrit | Merged openstack/oslo.config: Better check for integer range to account for 0 https://review.openstack.org/151940 | 18:34 |
| openstackgerrit | Ben Nemec proposed openstack/oslo.config: Log a warning when deprecated opts are used https://review.openstack.org/148020 | 18:41 |
| bnemec | dhellmann: Actually there is a circular dependency with versionutils, but it's okay because we don't need it at import time anyway. | 18:41 |
| dansmith | dims__: so our existing fixtures thing (obj_fixtures) is in tests/ and it's things the objects tests need | 18:43 |
| dansmith | dims__: just to be clear, you're suggesting having a fixture.py outside the tests directory, just s/checks/fixtures/ right? | 18:43 |
| dims__ | dansmith: yep | 18:43 |
| dims__ | these are fixtures to be used in other projects | 18:44 |
| dansmith | dims__: okay | 18:44 |
| openstackgerrit | Joshua Harlow proposed openstack/taskflow: Tweaks to atom documentation https://review.openstack.org/156844 | 18:47 |
| *** cdent has quit IRC | 18:52 | |
| dims__ | dansmith: checks.py has mock, but mock is only in test-requirements | 18:54 |
| dansmith | dims__: hmm, is it legit for me to add something like mock to requirements? | 18:55 |
| dims__ | trying to check other fixtures | 18:55 |
| dims__ | dansmith: only one we do that so far is oslotest | 18:56 |
| dansmith | dims__: hmm | 18:56 |
| *** achanda has joined #openstack-oslo | 18:56 | |
| *** e0ne has joined #openstack-oslo | 18:57 | |
| *** e0ne is now known as e0ne_ | 18:57 | |
| *** e0ne_ is now known as e0ne | 18:57 | |
| dims__ | dhellmann: wdyt? versionedobjects has a checks.py which needs mock (it has fixtures to be used by other projects). where do we specify mock library versions (requirements.txt vs test-requirements.txt) | 18:58 |
| *** vigneshvar has joined #openstack-oslo | 19:03 | |
| *** pradk has quit IRC | 19:07 | |
| dhellmann | bnemec: with the circular dep we can't have the 2 libraries depending on each other | 19:08 |
| dhellmann | dims__: what do we do with other libs fixtures and the fixtures library | 19:09 |
| dims__ | dhellmann: can't find any other fixture.py that needs mock | 19:09 |
| dhellmann | dims__: but they would all use the "fixtures" library, right? | 19:10 |
| dhellmann | dims__: https://pypi.python.org/pypi/fixtures | 19:10 |
| dhellmann | dims__: oslo.concurrency depends on fixtures directly | 19:12 |
| dims__ | dhellmann: ah | 19:13 |
| *** achanda has quit IRC | 19:14 | |
| dhellmann | I think since this is part of the public API of this library, the dependency can go into requirements.txt | 19:14 |
| dims__ | dansmith: checks.py imports fixtures, so we follow the same pattern as in oslo.concurrency | 19:14 |
| *** bknudson has quit IRC | 19:14 | |
| dims__ | dhellmann: ack | 19:14 |
| dhellmann | if we get push-back from anyone, we can put it in test-requirements.txt and just expect that projects that use it will install those as well | 19:14 |
| dhellmann | dims__: how about writing this up as a policy for our specs repo? | 19:14 |
| dansmith | dims__: so fixtures and mock need to go in requirements, right? | 19:15 |
| dims__ | dansmith: y | 19:15 |
| dhellmann | dansmith: yeah | 19:15 |
| dims__ | dhellmann: is there another spec review like this i can pattern it on? since this is not a code related thing | 19:16 |
| dansmith | okay cool | 19:17 |
| dhellmann | dims__: all of the ones in the policy subdir | 19:17 |
| dims__ | dhellmann: ack will look/submit | 19:17 |
| dhellmann | dims__: I just approved a bunch yesterday that had been up for review for a while without any -1 | 19:17 |
| openstackgerrit | Joshua Harlow proposed openstack/debtcollector: By default mutate the docstring(s) for deprecations https://review.openstack.org/155988 | 19:19 |
| openstackgerrit | Joshua Harlow proposed openstack/debtcollector: By default mutate the docstring(s) for deprecations https://review.openstack.org/155988 | 19:20 |
| *** achanda has joined #openstack-oslo | 19:21 | |
| openstackgerrit | Davanum Srinivas (dims) proposed openstack/oslo.versionedobjects: Cleanup comment and H803 hacking https://review.openstack.org/157114 | 19:25 |
| *** pradk has joined #openstack-oslo | 19:25 | |
| openstackgerrit | Joshua Harlow proposed openstack/debtcollector: By default mutate the docstring(s) for deprecations https://review.openstack.org/155988 | 19:27 |
| *** sreshetnyak has quit IRC | 19:31 | |
| openstackgerrit | Merged openstack/oslo.policy: Clean up configuration option management https://review.openstack.org/157044 | 19:39 |
| *** vigneshvar has quit IRC | 19:45 | |
| openstackgerrit | Joshua Harlow proposed openstack/debtcollector: By default mutate the docstring(s) for deprecations https://review.openstack.org/155988 | 19:46 |
| *** crc32 has joined #openstack-oslo | 19:48 | |
| openstackgerrit | Sridhar Gaddam proposed openstack/oslo.utils: Utility API to generate EUI-64 IPv6 address https://review.openstack.org/137774 | 19:56 |
| *** bknudson has joined #openstack-oslo | 19:59 | |
| *** andreykurilin_ has joined #openstack-oslo | 20:00 | |
| *** vigneshvar has joined #openstack-oslo | 20:02 | |
| openstackgerrit | Dan Smith proposed openstack/oslo.versionedobjects: Generalize compatibility testing https://review.openstack.org/157125 | 20:04 |
| openstackgerrit | Dan Smith proposed openstack/oslo.versionedobjects: Generalize the object relationships test https://review.openstack.org/157126 | 20:04 |
| openstackgerrit | Dan Smith proposed openstack/oslo.versionedobjects: Rename checks to fixture and update requirements https://review.openstack.org/157127 | 20:04 |
| dhellmann | bnemec: are you around to discuss the circular dep issue? I'm stumped. | 20:09 |
| *** achanda has quit IRC | 20:09 | |
| bnemec | dhellmann: I'm actually in a debugging session with someone right now. I'll let you know when I'm free. | 20:09 |
| *** exploreshaifali has quit IRC | 20:13 | |
| *** andreykurilin_ has quit IRC | 20:15 | |
| *** achanda has joined #openstack-oslo | 20:17 | |
| openstackgerrit | Davanum Srinivas (dims) proposed openstack/oslo-specs: Policy for specifying external library in requirements.txt https://review.openstack.org/157135 | 20:22 |
| bnemec | dhellmann: Okay, crisis averted for now. :-) | 20:26 |
| *** devlaps has quit IRC | 20:27 | |
| dhellmann | bnemec: so I have 3 alternatives for what we do with versionutils, and none seems great | 20:30 |
| dhellmann | 1. move it to oslo.utils, introducing a dependency between oslo.utils and oslo.config | 20:30 |
| dhellmann | 2. move it to its own library | 20:30 |
| dhellmann | 3. leave it in the incubator indefinitely | 20:30 |
| dhellmann | only option 3 lets us avoid a circular dependency between libraries :-/ | 20:31 |
| bnemec | dhellmann: Hmm, I'm not coming up with any terrific options either. | 20:34 |
| dhellmann | I guess alternative 4 is to put the code in oslo.config, but ew. | 20:35 |
| bnemec | We could copy just report_deprecated_feature into oslo.config, but then we have a potential opt mismatch if we ever change the versionutils opt definition. | 20:35 |
| dhellmann | right | 20:35 |
| dhellmann | the only options I'm coming up with for decoupling them involve complex callback registry schemes | 20:36 |
| dhellmann | bnemec: of all the times I think you would want to exit the app on a deprecation warning, a config option seems the most likely | 20:38 |
| bnemec | dhellmann: Maybe we could make the oslo.config dep in versionutils optional? Default to non-fatal if it's not present? | 20:38 |
| dhellmann | bnemec: hmm, or the other way around. If oslo.config can't import the versionutils code, it just uses LOG.warn() | 20:39 |
| bnemec | Yeah, that could also work. | 20:39 |
| harlowja_ | versionutils can't go in oslo.log? | 20:40 |
| dhellmann | harlowja_: oslo.config and oslo.log can't have a circular dependency either | 20:40 |
| openstackgerrit | Eric Brown proposed openstack/oslo.vmware: Resolve warnings from Bandit https://review.openstack.org/156889 | 20:40 |
| harlowja_ | hmmm | 20:40 |
| bnemec | harlowja_: The problem is I just changed https://review.openstack.org/#/c/148020/ to use versionutils. | 20:40 |
| *** yamahata has joined #openstack-oslo | 20:40 | |
| * dhellmann suggested it | 20:40 | |
| harlowja_ | hmmmmssss | 20:40 |
| bnemec | That change has a bad record with reviewer suggestions. They all turn out to be deep, deep rabbit holes. :-) | 20:41 |
| dhellmann | heh | 20:41 |
| dhellmann | the callback idea was to give oslo.config a function or method like set_deprecation_reporting_func() | 20:42 |
| harlowja_ | bnemec its cursed, i'll stay away, ha | 20:42 |
| dhellmann | the value would default to LOG.warn, but an app could pass in oslo.versionutils.whatever-that-function-si | 20:42 |
| dhellmann | but that feels a bit heavy-handed for one function | 20:42 |
| harlowja_ | dhellmann agreed, would work but does feel sorta odd | 20:43 |
| dhellmann | it also feels pretty odd to put an "I'm going to exit" call in a library | 20:43 |
| dhellmann | so maybe the right answer really is to just leave versionutils in the incubator indefinitely | 20:43 |
| bnemec | dhellmann: It doesn't exit though. It just raises an exception. | 20:44 |
| dhellmann | although that's sort of a technicality | 20:44 |
| harlowja_ | bnemec does anyone catch it :-P | 20:44 |
| dhellmann | oh, true, I guess I just assumed it wasn't being caught | 20:44 |
| bnemec | I'm betting there are lots of except Exceptions that would catch it. :-) | 20:44 |
| *** devlaps has joined #openstack-oslo | 20:44 | |
| dhellmann | true | 20:44 |
| bnemec | And it's off by default. The user has to explicitly ask for that behavior. | 20:45 |
| dhellmann | yeah | 20:46 |
| dhellmann | bnemec: ok, I give up. Let's go back to the version where you were just using LOG.warn() directly. It should be possible to recover that from gerrit so you don't have to reconstruct it. And if someone complains about the missing exception in the future we can think about it again. Maybe by then we'll have solved another case like this and have a pattern to follow. | 20:48 |
| bnemec | dhellmann: Sounds good. It's a lot better than the nothing we're doing with deprecated opts today. :-) | 20:49 |
| harlowja_ | put oslo.log,oslo.config into super repo+package called oslo-incubator2; profit :-P | 20:50 |
| dhellmann | harlowja_: it would have been technically easier to release the incubator as one big library, except maintenance would have been a nightmare | 20:51 |
| harlowja_ | :) | 20:51 |
| * dhellmann is startled to see blue sky and snow flurries through the same window | 20:52 | |
| openstackgerrit | Ben Nemec proposed openstack/oslo.config: Log a warning when deprecated opts are used https://review.openstack.org/148020 | 20:53 |
| dhellmann | bnemec: see my other comment on PS6 about the implementation of _check_deprecated() though, with the names | 20:54 |
| bnemec | Ah, yeah. I should have fixed that first. | 20:54 |
| bnemec | Would have saved me having to trick Gerrit into letting push a duplicate commit. :-) | 20:54 |
| dhellmann | :-) | 20:55 |
| dhellmann | dims__, stevemar, morganfainberg, ayoung : I think we're ready for an oslo.policy release. Is there any reason to wait? | 20:57 |
| *** mriedem1 has joined #openstack-oslo | 21:02 | |
| * dhellmann throws caution to the wind and releases oslo.policy 0.1.0 | 21:03 | |
| *** mriedem has quit IRC | 21:03 | |
| dims__ | dhellmann: yay! | 21:04 |
| dhellmann | hmm, our release notes script doesn't handle this case | 21:07 |
| dhellmann | dims__: I guess the last time we did a new lib we didn't announce the release to give us time to make sure the API worked out, right? | 21:09 |
| dims__ | dhellmann: yes | 21:09 |
| dhellmann | ok, we'll do the same thing here then | 21:09 |
| dims__ | we did that for oslo.log | 21:09 |
| dhellmann | no announcement email | 21:09 |
| dhellmann | right, that's the one | 21:09 |
| dhellmann | we have so many libraries now! | 21:09 |
| dims__ | dhellmann: there's a bug against oslo.policy asking where the release was, we could just close that for now | 21:10 |
| dims__ | someone eager to try it out, | 21:11 |
| dhellmann | I thought I already closed that as invalid | 21:11 |
| openstackgerrit | Ben Nemec proposed openstack/oslo.config: Log a warning when deprecated opts are used https://review.openstack.org/148020 | 21:11 |
| dims__ | ah ok dhellmann | 21:11 |
| dhellmann | dims__: I waffled, so I had to check, but yes, I did. | 21:12 |
| harlowja_ | u had a waffle | 21:13 |
| dhellmann | harlowja_: now I'm hungry | 21:14 |
| harlowja_ | me too | 21:14 |
| harlowja_ | lol | 21:14 |
| dhellmann | bnemec: isn't an empty group name the same as DEFAULT? | 21:15 |
| stevemar | \o/ | 21:16 |
| *** amrith is now known as _amrith_ | 21:16 | |
| bnemec | dhellmann: I don't think so: https://github.com/openstack/oslo.config/blob/master/oslo_config/cfg.py#L1538 | 21:17 |
| dhellmann | bnemec: I sure hope the default is None, then :-) | 21:18 |
| bnemec | dhellmann: Yeah, I was tempted to try it, but decided that was a rabbit hole too far. ;-) | 21:19 |
| dougwig | hi oslo. is this perhaps fallout from the namespace rename? | 21:20 |
| dougwig | https://www.irccloud.com/pastebin/hb74zcUv | 21:20 |
| dhellmann | dougwig: maybe? is that code in octavia? | 21:22 |
| openstackgerrit | Merged openstack/oslo-specs: Add revision history sections https://review.openstack.org/152618 | 21:22 |
| dhellmann | dougwig: it looks like maybe someone went a little too far with a sed conversion of oslo.config to oslo_config | 21:23 |
| dougwig | yes. i've never touched that except via oslo-incubator/update.py, and it seems that oslo_config got text replaced to oslo.config somehow | 21:23 |
| dhellmann | oh, hmm | 21:23 |
| dhellmann | maybe we need to fix up update.py then | 21:23 |
| openstackgerrit | Merged openstack/oslo-specs: Add adoption timeline to incubator policy https://review.openstack.org/153682 | 21:24 |
| dhellmann | dougwig: I can reproduce that when I run update.py. I'm looking at it. | 21:28 |
| dhellmann | dougwig: have you already filed a bug? | 21:28 |
| *** andreykurilin_ has joined #openstack-oslo | 21:32 | |
| openstackgerrit | Dan Krause proposed openstack/taskflow: add get_flow_details and get_atom_details to all backends https://review.openstack.org/157153 | 21:34 |
| openstackgerrit | Eric Brown proposed openstack/oslo.vmware: Resolve warnings from Bandit https://review.openstack.org/156889 | 21:34 |
| zzzeek | bnemec: when you say “We have multiple lines of communication available that don't require annoying all our users with unnecessary log messages.”, whats the line of communication for this kind of thing, the code doing something they should ideally correct for? how does an INFO message convey that this particular condition is less than ideal ? | 21:38 |
| bnemec | zzzeek: /join openstack-cinder? | 21:38 |
| bnemec | Or open a bug. Or send something to the list tagged [cinder] | 21:38 |
| zzzeek | bnemec: OK so, I should remove this patch then, and instead have it raise an error | 21:39 |
| zzzeek | ? | 21:39 |
| zzzeek | bsically youre saying , there’s no such class of behavior such as, “this is less than ideal but we can correct for it" | 21:39 |
| dougwig | dhellmann: not yet. want one? | 21:39 |
| bnemec | zzzeek: Right, we've corrected for it. We don't need to log a warning that's going to cause a bunch of operators to have to chase down the reason for it. | 21:40 |
| dhellmann | dougwig: yeah, if you have a minute. I think I have a fix for you but need to run some tests. | 21:40 |
| bnemec | Not when we can open a bug against cinder and say "fix this". | 21:40 |
| dougwig | ok, standby, meatspace intruding. | 21:40 |
| zzzeek | bnemec: what if we are correcting for many things that affect performance negatively? | 21:40 |
| zzzeek | bnemec: we just keep stuffing corrections in, hiding them, and thats that ? | 21:40 |
| bnemec | zzzeek: Tell the people who can fix it, not the operators. | 21:40 |
| bnemec | That's all I'm saying. | 21:41 |
| zzzeek | bnemec: and they say, “Theres no warning, theres nothing we can see, so we dont care” | 21:41 |
| * dhellmann pictures a side of beef barging into dougwig's office | 21:41 | |
| zzzeek | bnemec: also what about warnings.warn() ? | 21:41 |
| zzzeek | bnemec: sqlalhcemy has a crapload of warnings.warn() in it, all kinds of htings that might work the way someone is using them, but should be corrected | 21:41 |
| zzzeek | those should all be…hidden ? | 21:41 |
| bnemec | I'd be fine with warnings. They don't go to the logs by default. | 21:41 |
| zzzeek | bnemec: OK so can i cahgne this to warnings.warn() ? | 21:41 |
| zzzeek | bnemec: because it really isnt INFO and it really hsouldnt be hidden | 21:42 |
| bnemec | zzzeek: That's why I'm suggesting debug. That's the developer-oriented log level. | 21:42 |
| bnemec | But yeah, I'm fine with the warnings module. | 21:42 |
| bnemec | That seems developer oriented as well. | 21:42 |
| zzzeek | bnemec: who ever reads DEBUG, and if they did, how do they tell things that are “normal” from things that are errors? | 21:42 |
| bnemec | sdague: ^Just FYI, it seems we could use some more clarification on the target audience in the logging spec. | 21:45 |
| bnemec | We seem to have a difference of interpretation. | 21:46 |
| sdague | can someone summarize the question? | 21:47 |
| zzzeek | bnemec: I really dont care waht the interpretation is but that spec doesnt seem to make any room for “the code is doing a bad thing the developers should fix”. if it shoudl be DEBUG with a special prefix, great. whats the prefix ? | 21:47 |
| zzzeek | sdague: “the code is doing a bad thing that we can recover from, but the develoipers should fix it" | 21:47 |
| zzzeek | sdague: how to log ? | 21:47 |
| bnemec | Full discussion here: https://review.openstack.org/#/c/156725/2/oslo_db/sqlalchemy/session.py | 21:47 |
| sdague | which developers are the target? | 21:48 |
| zzzeek | sdague: poeple who commit to cinder, for example | 21:48 |
| zzzeek | sdague: someone who would make it so that they call engine.dispose() before launching a child process | 21:48 |
| zzzeek | well, wihtin the child proc. | 21:48 |
| zzzeek | warnings.warn() might be the most appropriate place | 21:49 |
| sdague | you are throwing an exception already though right? Isn't that the "don't do this thing" indication to developers | 21:49 |
| zzzeek | but warnings.warn() has the major issue that you cant put dynamic inforamtion in it without the chance of leaking memory | 21:49 |
| dougwig | dhellmann: lol | 21:49 |
| zzzeek | sdague: no, we can recover from it and continue. i can make it just throw instead, but that would be more disruptive | 21:49 |
| zzzeek | bnemec: in this specific case, we also could decide that this behavior is normal, i might be leaning towards that anyway. but im more curious about this warnings thing in general | 21:50 |
| bnemec | zzzeek: Yeah, it's a good discussion. | 21:51 |
| sdague | zzzeek: yeh, so here's the thing we've discovered. developers basically don't read the logs. | 21:51 |
| sdague | so it's not actually developer communication | 21:51 |
| zzzeek | sdague: well this is the thing with warnings, people complain about them, so they get noticed | 21:51 |
| sdague | what it actually turns into is "dear god, my cloud is busted" | 21:51 |
| zzzeek | sdague: sure | 21:51 |
| zzzeek | sdague: no middle ground huh | 21:51 |
| bnemec | Also, I actually think this behavior is broken anyway. Who knows what other file descriptors they've accidentally passed to child processes? | 21:52 |
| bnemec | Maybe we could add a flag to make this fatal in the gate? | 21:52 |
| zzzeek | bnemec: OK…how would they guard against that across-the-board ? | 21:52 |
| bnemec | That would get noticed by devs. :-) | 21:52 |
| sdague | bnemec: yeh, that's why I was wondering about the raise | 21:52 |
| dougwig | dhellmann: https://bugs.launchpad.net/oslo-incubator/+bug/1423361 | 21:53 |
| openstack | Launchpad bug 1423361 in oslo-incubator "incubator update.py converts oslo_config in cache module method name" [Undecided,New] | 21:53 |
| bnemec | zzzeek: Fork their workers before doing things like opening files and db connections? | 21:53 |
| sdague | because that seems to be the thing that we want to do, figure out how to signal the right audience to address it | 21:53 |
| zzzeek | this is really an odd one. these are database conecvtions, they’re buried in a connection pool people dont realize is there, and the patch easily just corrects for it by reopening the first time it hits in the child proc | 21:53 |
| zzzeek | bnemec: OK….but this seems to be some kind of ad-hoc process spawning thing that oslo.concurrency is doing | 21:53 |
| zzzeek | bnemec: it looks to be designed to work this way | 21:53 |
| zzzeek | bnemec: these arent long running processes | 21:54 |
| bnemec | zzzeek: Well, processutils was more intended for doing things like running iptables from a service. | 21:54 |
| zzzeek | bnemec: OK so, I can make this jsut raise. want me to do that? | 21:54 |
| zzzeek | bnemec: then cinder has to fix it on their end | 21:54 |
| zzzeek | bnemec: but that means, new oslo.db comes out, every app doing this somehwere breaks | 21:55 |
| bnemec | zzzeek: Yeah, exactly. | 21:55 |
| zzzeek | dhellmann: are you watching this convo | 21:55 |
| bnemec | If we want this to be fatal, I think it has to be _only_ in the gate. | 21:55 |
| bnemec | Like the fatal deprecations thing in versionutils. | 21:55 |
| zzzeek | bnemec: OK and if we’re not in the gate…then what ? silent recovery ? | 21:55 |
| * bnemec brings the conversation full circle ;-) | 21:56 | |
| sdague | bnemec: so, honestly, I personally think having the library not try to recover is the right philosophical thing to do | 21:56 |
| sdague | because I actually see issues like that pop up a bunch | 21:56 |
| sdague | like the embedded retry in execute | 21:56 |
| bnemec | I actually agree. This will also break horribly under py3. | 21:56 |
| *** e0ne has quit IRC | 21:56 | |
| bnemec | Unfortunately in py2 it's an intermittent failure, apparently. | 21:57 |
| zzzeek | bnemec: b.c. just any open FD will raise immediately ? | 21:57 |
| zzzeek | bnemec: if i have a proces, and i open an FD, then say fork(), it breaks immediately in py3? | 21:57 |
| bnemec | zzzeek: Because open fds get closed by default on forks. | 21:57 |
| bnemec | py3 sets O_CLOEXEC on file descriptors | 21:57 |
| zzzeek | bnemec: this recovery I have will work fine in py3k | 21:57 |
| zzzeek | bnemec: the descriptor is never touched | 21:57 |
| bnemec | zzzeek: Right, because you detect the problem and reopen it? | 21:58 |
| zzzeek | bnemec: i detect that the container for the conection was copied from a parent proc, i throw away the descriptor immediately and yes it has the invalidate flag set so it reopens immediately | 21:59 |
| sdague | zzzeek: if you do decide to leave the message in here, I'd get pretty explicit it, and figure out the process name you are running on and say "this is a bug with project XXXX, please file a bug at ... url" | 21:59 |
| zzzeek | sdague: OK what log level | 22:00 |
| sdague | if you get that explicit, WARN seems fine to me | 22:00 |
| zzzeek | DOH @ | 22:00 |
| zzzeek | heh | 22:00 |
| zzzeek | sdague: well thats waht the whole argument was about :) | 22:00 |
| * dims__ gets some popcorn :) | 22:00 | |
| * dhellmann reads scrollback | 22:00 | |
| * zzzeek has no opinion! does not care! | 22:01 | |
| sdague | zzzeek: right, but in order to do that, you need to really give the ops a straight line to the right dev place to complain | 22:01 |
| zzzeek | sdague: OK do you want to just make a comment on the review then, https://review.openstack.org/#/c/156725/2/oslo_db/sqlalchemy/session.py | 22:01 |
| sdague | because in it's current state as an op I'm going to look at it, not really understand if it's an issue, see that it happens a bunch, then whitelist it in my grep filters for not real issues | 22:01 |
| zzzeek | its between you and bnemec what you want to do, he was opposed to WARN | 22:01 |
| bnemec | Yeah, we did that before and got a lot of complaints. | 22:02 |
| sdague | bnemec: where did you used to do it? | 22:02 |
| bnemec | "Why are you warning me about this thing I can't fix?" | 22:02 |
| sdague | oh, yeh, that, agreed | 22:02 |
| zzzeek | you know what woudl be great. we decide what we want here. then we *update that spec* to be clear about this | 22:02 |
| sdague | if you are going to flag it that high you need to provide a path to fixing it | 22:02 |
| bnemec | That was the traditional sql mode thing, and we basically just turned it on by default and prayed. | 22:03 |
| bnemec | We got less complaints about that than the warning, oddly enough. | 22:03 |
| bnemec | Apparently we were already pretty traditional-safe. | 22:03 |
| sdague | bnemec: I know I bitched about that one :) | 22:03 |
| * sdague waiting for dhellmann to weigh in | 22:04 | |
| bnemec | Heh | 22:04 |
| * dhellmann has almost caught up | 22:04 | |
| * dhellmann wonders why walking away at 4:55 always results in so much scrollback | 22:04 | |
| haypo | dhellmann, harlowja_ : i wrote a new version of my asyncio spec, because i think that many people misunderstood the purpose of the spec. https://review.openstack.org/#/c/153298/3/specs/asyncio.rst | 22:04 |
| haypo | sorry, each version of the spec is longer than the previous spec! | 22:04 |
| dhellmann | sdague, bnemec, zzzeek : so the options are log a message intended for devs, use warnings.warn, or throw an exception? | 22:05 |
| haypo | the most interesting change is a new section on "race conditions" | 22:05 |
| zzzeek | dhellmann: or just consider this particular thing as “OK” | 22:05 |
| sdague | dhellmann: I think the options are | 22:05 |
| dhellmann | zzzeek: that's the "let the library recover" option? | 22:05 |
| sdague | 1) don't recover from the error, put the burden on the caller | 22:05 |
| zzzeek | dhellmann: yes. | 22:06 |
| haypo | harlowja_: ah yes, monkey-patched threading.RLock may work in eventlet | 22:06 |
| * dhellmann waits for sdague to finish enumerating | 22:06 | |
| sdague | 2) log something and recover | 22:06 |
| sdague | if 2) | 22:06 |
| sdague | then why are we logging? if it's because we are expecting someone to change something, we need to make sure it gets to the right audience | 22:07 |
| sdague | WARN "Invalidating connection detected as shared to new... " | 22:07 |
| dhellmann | what about logging and not recovering? then the log message will be in the job that fails | 22:07 |
| dhellmann | er, in the output for the job that fails | 22:07 |
| sdague | is mostly going to just confuse people reading the message | 22:07 |
| bnemec | Also, it sounds like this happens every run, but only intermittently causes failures. | 22:08 |
| dhellmann | yeah, let's assume we can come up with a message that isn't confusing to developers if we're going to log it and figure out the wording separately | 22:08 |
| zzzeek | bnemec: well for sure, it causes random race conditions | 22:08 |
| zzzeek | bnemec: but yes its on every run if certain options in cinder are turned on | 22:09 |
| zzzeek | bnemec: that make it go off and use oslo.concurrency when it starts up | 22:09 |
| bnemec | Yeah, so we need all or nothing. Either recover, or fail immediately. | 22:09 |
| zzzeek | OK well, that gerrit has “the code”. you all can make it do whatever :) | 22:09 |
| dhellmann | how about adding just the log message for now and cutting a release so we can measure how often it happens in projects other than cinder | 22:09 |
| zzzeek | dhellmann: OK. What level ? :) | 22:09 |
| dhellmann | that way we can tell whether it's safe to make it fail immediately | 22:10 |
| sdague | so I think that's actually the crux of the problem really | 22:10 |
| dhellmann | zzzeek: what level do we log at for developers in gate jobs? | 22:10 |
| zzzeek | dhellmann: shrugs ? | 22:10 |
| dhellmann | debug, I would think | 22:10 |
| sdague | because it's like "oh, this thing is wrong, it's your problem, except, I'll fix it for you" | 22:10 |
| zzzeek | dhellmann: OK ! | 22:10 |
| dhellmann | I don't know if we actually turn the logging up that high | 22:10 |
| zzzeek | dhellmann: debug it is | 22:10 |
| sdague | yes, we use DEBUG in the gate | 22:10 |
| zzzeek | dhellmann: any wording in the DEBUG that make it clear that “hey, you should probably see why this is happening” ? | 22:10 |
| zzzeek | dhellmann: like a prefix ? | 22:10 |
| bnemec | It's not safe to make it fail immediately. That will break all existing Cinder code if they happen to grab the new oslo.db. | 22:10 |
| zzzeek | and …can this be added to the spec ?? to avoid 30 min IRC discussion ? | 22:11 |
| dhellmann | sdague: right, I completely agree that if we could we'd just make it fail. However, if we know this is going to break things and piss people off, we should go a little more slowly. | 22:11 |
| dhellmann | zzzeek: I believe the spec already says that developer messages should be logged at debug | 22:11 |
| zzzeek | dhellmann: OK but how do we distinguish “normal operations” DEBUG from “HEY! this is BAD!” DEBUG ? | 22:11 |
| zzzeek | dhellmann: just need the lingo | 22:12 |
| zzzeek | dhellmann: if someoen wants to just pop it onto the gerrit here | 22:12 |
| dhellmann | zzzeek: and no, just write a clear message explaining what happened. Something like "This process forked while a database connection was open. That is likely to cause race conditions." | 22:12 |
| bnemec | Sort of serious suggestion: LOG.debug('WARNING: ...') | 22:12 |
| dhellmann | zzzeek: operators should not need to read debug messages | 22:12 |
| zzzeek | dhellmann: OK….but we recovered, so there wont be any race conditions | 22:12 |
| zzzeek | dhellmann: that is fine | 22:12 |
| sdague | zzzeek: I'd actually prefix with "BUG: " | 22:12 |
| zzzeek | dhellmann: OK, so verbiage here is….ah “BUG:”, sure | 22:12 |
| dhellmann | zzzeek: I'm suggesting that you remove the fix and leave the race condition for now | 22:12 |
| sdague | and I think it's a good point, and we should add to the spec | 22:12 |
| zzzeek | dhellmann: wow really. UM, wow. OK | 22:13 |
| dhellmann | so JUST log, and let's release a version of the lib that does that and see what we get in logstash | 22:13 |
| sdague | I'll throw a patch up tomorrow | 22:13 |
| dhellmann | zzzeek: hold your horses | 22:13 |
| zzzeek | hey i have to pick up my wife, takes 20 minutes can you folks update me via gerrit / etc? | 22:13 |
| dhellmann | I suggest a 2 phase approach. | 22:13 |
| bnemec | Couldn't we fix the race and log the message to see what we get in logstash? | 22:13 |
| dhellmann | or we can all talk at once | 22:13 |
| * zzzeek has to bbl, sorry | 22:13 | |
| dhellmann | phase 1: log only the condition that is the problem, without fixing it | 22:13 |
| dhellmann | phase 2: based on how many apps we find with that error, decide whether to fix it by recovering or introduce an exception | 22:14 |
| haypo | "zzzeek: Because open fds get closed by default on forks." *_CLOEXEC only ask to close the fd on exec(), not at fork | 22:14 |
| dhellmann | if we introduce an exception, then we go to the app(s) who will break and let them know so they can fix their code before we do that | 22:14 |
| sdague | dhellmann: debug is not indexed in logstash, so that plan isn't going to work | 22:14 |
| dhellmann | ok, well, then we'll have to log at info or something so we can measure it | 22:15 |
| dhellmann | we can change it to debug as part of phase 2 | 22:15 |
| sdague | the gate is only one access pattern though | 22:15 |
| dhellmann | true | 22:15 |
| sdague | so I don't think that's enough data to make the final decision on what to do, you should figure out where you'd like to be, and head there. | 22:15 |
| dhellmann | ok, well, it seems we want the error. I just don't want to deal with the fallout of introducing a fatal error like that knowingly. | 22:16 |
| sdague | mailing list thread? make sure to cc ptls of known effected projects? | 22:16 |
| dhellmann | we don't know which projects are affected, though, that's the point | 22:17 |
| dhellmann | so far we only know about cinder | 22:17 |
| bnemec | Cinder is the only known affected right now. | 22:17 |
| dhellmann | if it's only cinder, why is this even an oslo bug? | 22:17 |
| dhellmann | my guess is that we'll find other projects | 22:18 |
| dhellmann | can we find them through looking at code instead of running gate jobs? | 22:18 |
| dims__ | dhellmann: i'd suspect neutron as well | 22:18 |
| bnemec | It's not really an oslo bug, but it ends up looking like one. | 22:18 |
| sdague | so if you expose on the ML list as a more general thread, maybe you'll get other people chiming in | 22:18 |
| sdague | or paying attention, I could be overly optimistic there | 22:18 |
| bnemec | Probably :-) | 22:19 |
| dhellmann | sdague: yeah, I'm a bit pessimistic about that but it's worth a try | 22:19 |
| sdague | but that does seem like the cross dev community communication bus | 22:19 |
| dhellmann | I'd like to at least agree on an approach to put in that email,t hough | 22:19 |
| dhellmann | and frankly, I expect most people to want the faster horse instead of the car | 22:19 |
| dhellmann | i.e., recover quietly and leave me alone | 22:20 |
| bnemec | Yeah, same thing with the monkey patching | 22:20 |
| dims__ | dhellmann: my vote would be on "don't recover from the error, put the burden on the caller" | 22:20 |
| sdague | dhellmann: maybe, it ends up hiding a ton of misuse issues | 22:20 |
| dhellmann | sdague: oh, I agree, I'm just making predictions | 22:20 |
| haypo | zzzeek: "Cinder is spawning a subprocess using oslo.concurrency and is failing to call create_engine() local to that process" i don't understand. the child process inherits file descriptors? does the child process use these file descriptors? | 22:20 |
| sdague | yeh, my inclinations is that libraries shouldn't do recovery like this | 22:20 |
| dims__ | +1 | 22:21 |
| dhellmann | haypo: yeah, the child process is reusing the open database connection | 22:21 |
| dhellmann | sdague: right, it doesn't tend to work out well doing it at that low a level | 22:21 |
| haypo | dhellmann: how does the child process pick the file descriptor if it is now aware that it inherited unwanted file descriptors? | 22:21 |
| dhellmann | bnemec: can you start the mailing list thread? you have more background on this than I do | 22:21 |
| bnemec | Ooh, what about a deprecation period for the current behavior? | 22:21 |
| dhellmann | haypo: https://review.openstack.org/#/c/156725/2/oslo_db/sqlalchemy/session.py | 22:22 |
| bnemec | We start by logging the warning, then next cycle or whenever we make this fail fast? | 22:22 |
| dhellmann | bnemec: I think we want the bug fixed this cycle, though | 22:22 |
| dims__ | bnemec: 1 release of oslo.db, not one full cycle | 22:22 |
| bnemec | We can do the current workaround during the deprecation period. | 22:22 |
| bnemec | dims__: I'd want a full cycle to give people a chance to find and fix all the problems. | 22:22 |
| * bnemec hears crickets | 22:24 | |
| dhellmann | I'm not keen on a long grace period for this fix. | 22:24 |
| dims__ | bnemec: does not help, if people don't heed the warning, then they will never change | 22:25 |
| dims__ | they will change only when we break them | 22:25 |
| dhellmann | I really do want more info about how many projects have the issue, though, because if it's just one then the answer becomes much simpler. | 22:25 |
| dims__ | right dhellmann | 22:25 |
| bnemec | dims__: Right, but oslo releases can happen within a couple of weeks. | 22:25 |
| dims__ | bnemec: we can hold the line on oslo.db | 22:25 |
| bnemec | Yeah, until we merge another critical bug fix. :-) | 22:26 |
| dims__ | haha | 22:26 |
| dhellmann | I also don't want to set the precedent that we're going to hold up releases for an extended period of time for situations like this. | 22:27 |
| *** jecarey has quit IRC | 22:28 | |
| bnemec | I guess my thing is that I don't think we can break people without any warning, even if Cinder does turn out to be the only one. | 22:28 |
| dhellmann | bnemec: did you agree to start that email thread? | 22:29 |
| dhellmann | bnemec: if cinder is the only case where this happens, then we don't need to make any changes other than the log message in the db library | 22:30 |
| dims__ | bnemec: mine is that we need to get this under control before we ship out kilo even if we have to break them in the interim, dont want known issues like this in production | 22:30 |
| bnemec | dhellmann: I guess so | 22:30 |
| dhellmann | we could, but we don't *need* to | 22:30 |
| *** achanda has quit IRC | 22:30 | |
| bnemec | I'm still not clear that we have a consensus on an approach though. Are we going to start with only the log message? | 22:31 |
| * harlowja_ man this is a lively conversation, lol | 22:31 | |
| dims__ | :) harlowja_ | 22:32 |
| bnemec | Logging. Serious business. | 22:32 |
| dims__ | haha bnemec | 22:32 |
| harlowja_ | ;) | 22:32 |
| haypo | dhellmann: i read the change but i don't understand how a connection record can be inherited from a parent to a child process when the subprocess module | 22:32 |
| dhellmann | bnemec: that's what we need the thread to figure out, I guess. | 22:33 |
| harlowja_ | haypo i think its because sqlalchemy engines are pooled (?) | 22:33 |
| *** noelbk has quit IRC | 22:33 | |
| dhellmann | haypo: I'm not sure they are using subprocess(), but you'd have to ask zzzeek because he's the one that found the problem | 22:33 |
| bnemec | dhellmann: Okay, I'll just try to list the options we've discussed then. | 22:33 |
| dhellmann | bnemec: yeah | 22:34 |
| harlowja_ | if its just subprocess why not just off all open file descriptors? | 22:34 |
| * bnemec puts on his flame retardant suit just in case he gets any of them wrong ;) | 22:34 | |
| dhellmann | haypo: this thread may have more detail: http://lists.openstack.org/pipermail/openstack-dev/2015-February/057097.html | 22:34 |
| * harlowja_ goes back to being quiet (to much detail i dont know about, ha) | 22:35 | |
| harlowja_ | ^ be quiet josh, ha | 22:35 |
| dhellmann | harlowja_: the app should be doing that, but isn't | 22:35 |
| dhellmann | that's the actual bug | 22:35 |
| haypo | dhellmann: i guess that cinder uses fork, not subprocess.Popen | 22:35 |
| haypo | dhellmann: thanks for the links | 22:36 |
| dhellmann | haypo: that may be the case | 22:36 |
| bnemec | haypo: Specifically this http://lists.openstack.org/pipermail/openstack-dev/2015-February/057184.html | 22:36 |
| bnemec | The eventlet stuff at the beginning of the thread seems to have been a red herring. | 22:37 |
| harlowja_ | i've seen stuff like https://github.com/thesharp/daemonize/blob/master/daemonize.py#L116 (wonder why we aren't doing something similar) | 22:37 |
| harlowja_ | or * https://github.com/BlueDragonX/detach/blob/master/detach.py#L61 (and many others) | 22:39 |
| openstackgerrit | Doug Hellmann proposed openstack/oslo-incubator: Fix the regex for turning project.lib back to oslo.lib https://review.openstack.org/157178 | 22:39 |
| harlowja_ | ^ all little fork 'helpers' | 22:40 |
| *** noelbk has joined #openstack-oslo | 22:40 | |
| haypo | oh, i remember with sileht explained me that the Service class is based on fork() and does crazy things to workaround fork() issues | 22:42 |
| harlowja_ | ya, but i wonder why we didn't follow similar patterns to other daemonizeing things i've seen (that afaik all by default close fds) | 22:42 |
| *** mriedem1 has quit IRC | 22:42 | |
| harlowja_ | maybe something idk (or some history there) | 22:42 |
| haypo | harlowja_: Service design is completly crazy :) | 22:43 |
| harlowja_ | haypo no disagreement there :-P | 22:43 |
| harlowja_ | some crazy history is highly likely, lol | 22:43 |
| haypo | since service.py is still in the incubator, it's not too late to fix it ;) | 22:44 |
| dhellmann | well, aren't there cases where forking and keeping the file descriptors open is valid? | 22:44 |
| harlowja_ | dhellmann probably, but maybe that should be the exception and not the default? | 22:44 |
| dhellmann | harlowja_: quite possibly | 22:45 |
| harlowja_ | although i do wonder how cinder is executing code before service.py does stuff | 22:45 |
| dhellmann | bnemec: fixing the service module to close file descriptors after a fork by default is another approach we could take. That would let apps sync the fix when they are ready, rather than when we release oslo.db. | 22:45 |
| harlowja_ | most usages i've seen the service.py stuff is the first thing to run (and nothing else runs before it) | 22:45 |
| harlowja_ | ya, shutoff all file-descriptors (add exceptions to this as needed) | 22:46 |
| *** achanda has joined #openstack-oslo | 22:46 | |
| harlowja_ | aw, sweet, we shoudl use this one | 22:46 |
| harlowja_ | https://github.com/Gamocosm/minecraft-server_wrapper/blob/master/daemon.py#L62 | 22:46 |
| harlowja_ | and turn service.py into a minecraft server when its idle | 22:47 |
| harlowja_ | lol | 22:47 |
| harlowja_ | ^ joke... | 22:47 |
| haypo | anyway, i contributed to https://bugs.launchpad.net/cinder/+bug/1417018 with my theory on fork | 22:48 |
| openstack | Launchpad bug 1417018 in Cinder "Cinder encounters dbapi error: NoSuchColumnError: "Could not locate column in row for column '%(140070887032400 anon)s.volumes_id'"" [Undecided,New] - Assigned to Mike Bayer (zzzeek) | 22:48 |
| *** crc32 has quit IRC | 22:48 | |
| bnemec | dhellmann: Yeah, if we could fix it in the service code that would be nice. | 22:49 |
| bnemec | Especially since that hasn't graduated yet. | 22:49 |
| dhellmann | right | 22:49 |
| dhellmann | zzzeek doesn't say which process in cinder has the issue | 22:49 |
| zzzeek | dhellmann: it has to do with ceph | 22:50 |
| zzzeek | dhellmann: i had some log output in the email thread | 22:50 |
| dhellmann | zzzeek: which service process was it? | 22:50 |
| * harlowja_ trying to connect it to https://github.com/openstack/cinder/tree/master/cinder/cmd (and which one here) | 22:50 | |
| harlowja_ | hmmm, i guess it could be https://github.com/openstack/cinder/blob/master/cinder/cmd/volume.py#L62 | 22:51 |
| *** yamahata_ has quit IRC | 22:51 | |
| zzzeek | dhellmann: CMD "sudo cinder-rootwrap /etc/cinder/rootwrap.conf env LC_ALL=C LVM_SYSTEM_DIR=/etc/cinder vgs --noheadings --unit=g -o name,size,free,lv_count,uuid --separator : --nosuffix stack-volumes-lvm-1" | 22:51 |
| harlowja_ | 'server = service.Service.create' activating different backends (that may be doing bad things) | 22:51 |
| *** isq has joined #openstack-oslo | 22:51 | |
| harlowja_ | rootwrap stuff all goes through subprocess right? | 22:51 |
| harlowja_ | *via execute() | 22:51 |
| harlowja_ | *via process utils execute | 22:52 |
| haypo | harlowja_: the "CMD ..." log comes from execute() which uses subprocess.Popen with close_fds=True | 22:52 |
| dhellmann | zzzeek: which process was calling that rootwrap command, I mean | 22:52 |
| zzzeek | dhellmann: hard to say | 22:52 |
| dhellmann | I'm trying to understand which daemon it is, so I can look at that daemon's startup code | 22:52 |
| zzzeek | dhellmann: oh | 22:52 |
| zzzeek | dhellmann: well its cinder-volume | 22:53 |
| zzzeek | dhellmann: i have step-by-step in the bug report | 22:53 |
| zzzeek | to reproduce | 22:53 |
| dhellmann | zzzeek: the bug isn't linked from https://review.openstack.org/#/c/156725/2 is it in the email thread somewhere? | 22:54 |
| zzzeek | dhellmann: oh. hm | 22:56 |
| dhellmann | hmm, this looks like a fundamental issue with the way service works | 22:56 |
| dhellmann | create() takes the name of a manager class, and the instantiates it | 22:56 |
| zzzeek | dhellmann: its in one of my comments there its https://bugs.launchpad.net/cinder/+bug/1417018 | 22:56 |
| dhellmann | in __init__ | 22:56 |
| openstack | Launchpad bug 1417018 in Cinder "Cinder encounters dbapi error: NoSuchColumnError: "Could not locate column in row for column '%(140070887032400 anon)s.volumes_id'"" [Undecided,New] - Assigned to Mike Bayer (zzzeek) | 22:56 |
| dhellmann | zzzeek: next revision it would be good to add that to the commit message | 22:57 |
| zzzeek | dhellmann: yeah i didnt know the system to add bugs from a different project? | 22:57 |
| zzzeek | dhellmann: just full hyperlink ? | 22:57 |
| dhellmann | zzzeek: no, just "Closes-Bug: #xxx" or whatever | 22:57 |
| zzzeek | dhellmann: OK but that bug number is local to cinder, not oslo | 22:57 |
| dhellmann | launchpad bugs can be associated with more than one project | 22:58 |
| zzzeek | dhellmann: cross-project bug linking basically | 22:58 |
| zzzeek | dhellmann: OK | 22:58 |
| zzzeek | dhellmann: but..this is an oslo.db bug ? :) | 22:58 |
| dhellmann | zzzeek: reload the bug report, now it's associated with both cinder and oslo.db | 22:58 |
| zzzeek | dhellmann: yes. so this is an oslo.db bug also? | 22:58 |
| zzzeek | dhellmann: menaing, “oslo.db does not guard against XYZ” ? | 22:59 |
| haypo | zzzeek: the root cause should be investigated IMO | 22:59 |
| dhellmann | zzzeek: no, not really. I think the right thing to do here is fix the service code so we close file descriptors before forking | 22:59 |
| zzzeek | haypo: the root cause is exactly known | 22:59 |
| haypo | zzzeek: ah? what is it? | 22:59 |
| dhellmann | zzzeek: in general I don't like the libraries trying to recover from issues that came from elsewhere | 22:59 |
| zzzeek | haypo: what I stated in the bug report | 22:59 |
| *** openstackgerrit has quit IRC | 23:00 | |
| zzzeek | haypo: file descriptor is traveling to a new child process | 23:00 |
| dhellmann | zzzeek: I don't think it's that subprocess call, though. | 23:00 |
| haypo | zzzeek: see my comment: i don't undestand your analysis | 23:00 |
| *** openstackgerrit has joined #openstack-oslo | 23:00 | |
| zzzeek | dhellmann: well if I make that call fail, then the bug goes away | 23:00 |
| dhellmann | subprocess closes file descriptors | 23:00 |
| zzzeek | haypo: why dont you run my step-by-step reproucition steps, pdb into it and let me know what you think ? | 23:00 |
| zzzeek | dhellmann: does it close FDs that are open by native -C code like MySQLdb ? | 23:01 |
| zzzeek | dhellmann: but actually i can reproduce it with pymysql also | 23:01 |
| haypo | zzzeek: it's around midnight, i'm not supposed to work :) maybe tomorrow if you can help me to prepare the setup to reproduce the issue | 23:01 |
| zzzeek | dhellmann: bnemec tells me subproces only closes FDs in python 3 | 23:01 |
| haypo | zzzeek: yes, close_fds=True closes all file descriptors | 23:01 |
| zzzeek | haypo: the steps are all there, do you know how to run devstack ? | 23:01 |
| haypo | zzzeek: sure, i have a devstack which is supposed to work :-) | 23:01 |
| dhellmann | zzzeek: http://git.openstack.org/cgit/openstack/oslo.concurrency/tree/oslo_concurrency/processutils.py#n213 | 23:02 |
| haypo | zzzeek: oslo.concurrency sets explicitly close_fds to True, so it also close FDs on Python 2 | 23:02 |
| zzzeek | haypo: OK, so the bug does not exist, lets close it ! :) | 23:02 |
| haypo | that's why i'm asking if the issue doesn't come from fork | 23:02 |
| zzzeek | haypo: whats the other way for a linux proces to make a child process besides fork ? | 23:03 |
| haypo | zzzeek: Popen uses fork+exec. the Service class only uses fork | 23:03 |
| dhellmann | zzzeek: I suspect this is our real issue: http://git.openstack.org/cgit/openstack/oslo-incubator/tree/openstack/common/service.py#n308 | 23:03 |
| dhellmann | because the service class is set up with a database connection before the launcher forks | 23:03 |
| haypo | dhellmann: i love fork so much | 23:03 |
| * dhellmann should be putting a fork into his dinner soon | 23:04 | |
| * haypo prefers spoons | 23:04 | |
| dhellmann | zzzeek: for example, if cinder-volume starts multiple workers, they would *all* share that initial database connection | 23:04 |
| * bnemec might be on a liquid diet after this discussion | 23:04 | |
| zzzeek | dhellmann: I only narrow it down to oslo.concurrency b.c. if i make that specific part of it fail, the issue goes away, and the error always occurs right after that log line in oslo.concurrency with “CMD” | 23:05 |
| harlowja_ | https://hg.python.org/releasing/2.7.9/file/753a8f457ddc/Lib/subprocess.py#l1181 (looks similar to what the other daemon code i've seen do) ; looks ok in 2.7 at least, ha | 23:05 |
| zzzeek | if you all like, I can log the process id in oslo.concurrency there and prove its the same number in the error condition | 23:05 |
| dhellmann | zzzeek: yeah, I believe you, I'm just not sure I understand why it happens there | 23:05 |
| zzzeek | dhellmann: gonna say close_fds does not do waht we think it does in py2k or the flag is not set to True | 23:05 |
| dhellmann | that would be interesting to see, too. Maybe cinder is passing false for that. | 23:06 |
| haypo | dhellmann: "if cinder-volume starts multiple workers ..." do you mean the at application class is created once and then we get multiple processes using os.fork? | 23:06 |
| bnemec | It's not configurable. The only time it's false is on windows. | 23:06 |
| dhellmann | zzzeek: how many workers does devstack create for cinder-volume with that configuration? | 23:06 |
| haypo | dhellmann: that's completly insane. how is it possible that it worked before? :) | 23:06 |
| zzzeek | dhellmann: haven’t analyzed that. seemed like it was doing “a bunch” | 23:06 |
| harlowja_ | https://hg.python.org/releasing/2.7.9/file/753a8f457ddc/Lib/subprocess.py#l1238 is all the stuff that happens in 2.7 in the child | 23:07 |
| dhellmann | haypo: ah, I hadn't noticed that | 23:07 |
| * zzzeek has just this one point. He ran the whole thing from scratch to see it happening, and he put the full steps to reprocue. so if folks want to confirm, please run the steps | 23:07 | |
| dhellmann | oops, that was for bnemec ^^ | 23:07 |
| dhellmann | zzzeek: do you have any reports of this problem in projects other than cinder? | 23:08 |
| zzzeek | dhellmann: nope | 23:08 |
| *** pradk has quit IRC | 23:08 | |
| dhellmann | ok, that's something | 23:08 |
| zzzeek | dhellmann: however, the symptoms are extremely obtuse and distant from the acvtual problem | 23:09 |
| zzzeek | dhellmann: very occasional MySQL screwups | 23:09 |
| haypo | dhellmann: a friend told me that after fork, the POSIX standard only allows a very low number of syscalls, something like read, write, exec | 23:09 |
| dhellmann | so we outlined a few options, and bnemec is going to start a mailing list thread on it. I'm inclined to leave this as a cinder bug and not have the library recover from the problem. I know it's easy, but I don't want to hide issues like this. | 23:09 |
| zzzeek | dhellmann: and in fact here its quite difficult to reproduce. I had to patch cinder to run the one query 100 times in a row to get the race to collide consistently | 23:09 |
| dhellmann | I'm not sure if I want to throw an error or not | 23:09 |
| haypo | dhellmann: fork is only supposed to be used to execute fork, that's all :) | 23:09 |
| haypo | oops | 23:09 |
| haypo | dhellmann: fork is only supposed to be used to call exec(), that's all :) | 23:09 |
| harlowja_ | maybe zzzeek your machine got bombarded by space rays when this happened? | 23:09 |
| harlowja_ | and it flipped a bit | 23:09 |
| zzzeek | harlowja_: and the two other folks who reported the issue, yup | 23:10 |
| dhellmann | haypo: that's not true, processes fork() to daemonize themselves all the time | 23:10 |
| harlowja_ | space rays could of hit them too zzzeek | 23:10 |
| *** daniel3_ has quit IRC | 23:10 | |
| dhellmann | zzzeek: for the record, I believe there is a problem and that it's related to forking somehow and that your patch would "fix it" -- I'm just not sure that's the best solution long-term. I'd like to understand why this is only coming up in cinder, for example. That's why I suggested that we start by logging the situation and measure where we see it and how often. | 23:11 |
| haypo | dhellmann: ah maybe. but instanciate the application after the fork would avoid many issues | 23:11 |
| bnemec | dhellmann: Do we still want the thread now, or do we want to look into the Service thing? Because if we can fix it in Service then it may affect my preferences. | 23:12 |
| dhellmann | haypo: yes, I agree. I'll have to try to wrap my head around the service code better to see if that's possible. | 23:12 |
| harlowja_ | its probably a good idea to have service.py shutoff fds right? | 23:12 |
| zzzeek | dhellmann: okey doke, as I’ve maintained since….yesterday, I can make it A. raise B. do nothing C. correct for it, D. choose not to decide but we stil have made a choice, so... | 23:12 |
| dhellmann | bnemec: let's mention the service option, but I think that's going to be a longer-term fix no matter what. The folks that wrote that code aren't around any more | 23:12 |
| harlowja_ | *since that seems to match the behavior of all other libraries that fork (and subprocess itself) | 23:12 |
| * zzzeek does not care! | 23:12 | |
| harlowja_ | zzzeek u should do Z | 23:13 |
| harlowja_ | option Z please, thx | 23:13 |
| * zzzeek does the happy this-is-not-his-code dance | 23:13 | |
| dhellmann | zzzeek: I'm leaning towards B' (do nothing other than log a debug message) but I just don't want you to get frustrated by us taking some time to understand the issue | 23:13 |
| * harlowja_ runs aawy | 23:13 | |
| zzzeek | dhellmann: i dont need this issue fixed. a coworker asked me to look at it, and it was totalyl mysterious and disturbing, like the connection pool was broke or something, so im jstu super glad its nothing on my end :) | 23:14 |
| dhellmann | zzzeek: cool, like I said, just making sure the delay isn't annoying :-) | 23:14 |
| zzzeek | id encourgage everyone to jump on the bug report and do their worst | 23:14 |
| haypo | FYI I'm interested to help to redesign the service class. but it may be difficult to change it in stable branches | 23:14 |
| haypo | so zzzeek specific issue maybe need other changes in stable branches | 23:15 |
| zzzeek | haypo: if other proejcts are forking and not disposing their engines and we decide not to correct for it in the pool, then yes | 23:15 |
| haypo | we may need a new service class which has a sane design, to not break applications using the old class | 23:15 |
| haypo | zzzeek: i don't know what is the best place to workaround the fork issue, i don't know enough how SQLAchemy is used | 23:16 |
| haypo | i have to go, bye | 23:17 |
| dhellmann | it's time for me to head out, too | 23:18 |
| bnemec | Good night everyone. | 23:19 |
| harlowja_ | i'm still around (now all lonely) :( | 23:23 |
| *** noelbk has quit IRC | 23:24 | |
| *** noelbk has joined #openstack-oslo | 23:28 | |
| *** dims_ has joined #openstack-oslo | 23:49 | |
| *** andreykurilin_ has quit IRC | 23:50 | |
| *** dims__ has quit IRC | 23:51 | |
| *** vigneshvar has quit IRC | 23:51 | |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!