Wednesday, 2015-02-18

kragnizthanks for the help00:00
harlowja_np00:00
*** jaosorior has quit IRC00:01
openstackgerritMerged openstack/oslo.config: Fixes deprecation warning for oslo.config in cfg.py  https://review.openstack.org/15673000:06
openstackgerritMerged openstack/oslo.config: Support i18n messages in config generator  https://review.openstack.org/14528000:15
openstackgerritMerged openstack/oslo.log: Update Oslo imports to remove namespace package  https://review.openstack.org/14909000:26
*** jaypipes has quit IRC00:27
himangidhellmann: I have got myself setup00:30
*** dims__ has quit IRC00:33
*** dims__ has joined #openstack-oslo00:33
*** dims__ has quit IRC00:33
*** dims__ has joined #openstack-oslo00:35
openstackgerritMerged openstack/oslo.config: Add a list_all_sections method to ConfigOpts  https://review.openstack.org/14893600:37
himangiwhich is the development repository for oslo?00:37
*** bknudson has quit IRC00:41
*** david-lyle has quit IRC00:44
*** hemna is now known as hemnafk01:08
*** achanda has quit IRC01:17
*** zzzeek has quit IRC01:22
*** zzzeek has joined #openstack-oslo01:31
*** sigmavirus24 is now known as sigmavirus24_awa01:38
*** salv-orlando has quit IRC01:58
*** stevemar has quit IRC02:09
*** david-lyle has joined #openstack-oslo02:11
*** mriedem has joined #openstack-oslo02:14
*** mriedem has quit IRC02:15
*** mriedem has joined #openstack-oslo02:16
openstackgerritJoshua Harlow proposed openstack/taskflow: WIP for having the ability to have tooz find workers+topics  https://review.openstack.org/15149502:19
*** stevemar has joined #openstack-oslo02:22
*** takedakn has joined #openstack-oslo02:24
*** takedakn has quit IRC02:26
*** noelbk has quit IRC02:26
*** takedakn has joined #openstack-oslo02:26
*** tsekiyam_ has joined #openstack-oslo02:33
*** tsekiyama has quit IRC02:37
*** tsekiyam_ has quit IRC02:38
*** mtanino has quit IRC02:43
*** takedakn has quit IRC02:48
*** salv-orlando has joined #openstack-oslo02:59
openstackgerritMerged openstack-dev/hacking: H105: also check for Authors and authors  https://review.openstack.org/14720103:03
openstackgerritMerged openstack/oslo-incubator: Link hacking guidelines into dev docs  https://review.openstack.org/15223803:07
openstackgerritMerged openstack/oslo-incubator: Have a little fun with release notes  https://review.openstack.org/15268003:08
*** boris-42 has quit IRC03:12
*** yamahata has quit IRC03:13
*** himangi has quit IRC03:21
*** harlowja_ is now known as harlowja_away03:22
*** rushiagr_away is now known as rushiagr03:28
*** david-lyle has quit IRC03:35
*** david-lyle has joined #openstack-oslo03:35
*** david-lyle has quit IRC03:40
*** mriedem has left #openstack-oslo03:42
*** mriedem has quit IRC03:42
*** tsekiyama has joined #openstack-oslo03:44
openstackgerritMerged openstack/oslo.config: Do not import our namespace package  https://review.openstack.org/15682003:48
*** tsekiyama has quit IRC03:48
*** dims__ has quit IRC03:49
*** harlowja_at_home has joined #openstack-oslo03:50
*** salv-orlando has quit IRC03:51
*** amotoki has joined #openstack-oslo03:54
openstackgerritJoshua Harlow proposed openstack/taskflow: Tweaks to atom documentation  https://review.openstack.org/15684404:16
*** takedakn has joined #openstack-oslo04:19
*** stevemar has quit IRC04:20
*** harlowja_at_home has quit IRC04:21
*** david-lyle has joined #openstack-oslo04:22
*** david-lyle is now known as david-lyle_afk04:23
*** achanda has joined #openstack-oslo04:29
*** takedakn has quit IRC04:33
*** stevemar has joined #openstack-oslo04:36
*** achanda has quit IRC04:45
openstackgerritMerged openstack/taskflow: Add todo note for kombu pull request  https://review.openstack.org/15647204:46
*** dims__ has joined #openstack-oslo04:50
openstackgerritEric Brown proposed openstack/oslo.vmware: Add bandit to tox for security static analysis  https://review.openstack.org/15683804:52
*** achanda has joined #openstack-oslo04:52
*** dims__ has quit IRC04:54
*** achanda has quit IRC04:59
*** achanda has joined #openstack-oslo05:00
openstackgerritMerged openstack/taskflow: Improve multilock class and its associated unit test  https://review.openstack.org/15566305:02
*** achanda has quit IRC05:11
*** yamahata has joined #openstack-oslo05:33
*** salv-orlando has joined #openstack-oslo05:36
*** zzzeek has quit IRC05:43
openstackgerritEric Brown proposed openstack/oslo.vmware: Resolve warnings from Bandit  https://review.openstack.org/15688905:50
*** achanda has joined #openstack-oslo05:53
openstackgerritOpenStack Proposal Bot proposed openstack/oslo.db: Imported Translations from Transifex  https://review.openstack.org/15689006:01
openstackgerritJoshua Harlow proposed openstack/taskflow: Use the enum library for the retry strategy enumerations  https://review.openstack.org/15689306:02
*** crc32 has quit IRC06:03
openstackgerritOpenStack Proposal Bot proposed openstack/oslo.vmware: Imported Translations from Transifex  https://review.openstack.org/15689706:04
*** ajo_ has joined #openstack-oslo06:08
openstackgerritJoshua Harlow proposed openstack/taskflow: Use the enum library for the retry strategy enumerations  https://review.openstack.org/15689306:09
*** ajo has quit IRC06:11
openstackgerritJoshua Harlow proposed openstack/taskflow: Use the enum library for the retry strategy enumerations  https://review.openstack.org/15689306:14
*** achanda has quit IRC06:20
*** achanda has joined #openstack-oslo06:22
*** exploreshaifali has joined #openstack-oslo06:24
*** achanda has quit IRC06:32
*** exploreshaifali has quit IRC06:33
*** ajo_ is now known as ajo06:35
*** miqui has joined #openstack-oslo06:41
*** achanda has joined #openstack-oslo06:56
*** takedakn has joined #openstack-oslo07:10
*** exploreshaifali has joined #openstack-oslo07:18
*** vigneshvar has joined #openstack-oslo07:31
*** miqui has quit IRC07:39
*** miqui has joined #openstack-oslo07:41
*** takedakn has quit IRC07:48
*** miqui has quit IRC07:49
*** salv-orlando has quit IRC07:51
*** andreykurilin_ has joined #openstack-oslo07:53
*** takedakn has joined #openstack-oslo07:58
*** boris-42 has joined #openstack-oslo08:00
*** miqui has joined #openstack-oslo08:06
*** takedakn has quit IRC08:07
*** takedakn has joined #openstack-oslo08:15
*** yamahata has quit IRC08:15
*** yamahata has joined #openstack-oslo08:16
*** ajo has quit IRC08:18
*** rushiagr is now known as rushiagr_away08:21
*** viktors|afk is now known as viktors08:27
*** rushiagr_away is now known as rushiagr08:28
*** miqui has quit IRC08:30
*** stevemar has quit IRC08:31
*** achanda has quit IRC08:31
*** inc0 has joined #openstack-oslo08:33
*** himangi has joined #openstack-oslo08:34
*** ajo has joined #openstack-oslo08:35
*** salv-orlando has joined #openstack-oslo08:48
*** andreykurilin_ has quit IRC08:54
*** ajo has quit IRC08:56
*** salv-orlando has quit IRC09:01
*** ajo has joined #openstack-oslo09:01
*** salv-orlando has joined #openstack-oslo09:01
*** i159 has joined #openstack-oslo09:04
*** salv-orlando has quit IRC09:10
*** salv-orlando has joined #openstack-oslo09:10
*** exploreshaifali has quit IRC09:17
*** dulek has joined #openstack-oslo09:18
openstackgerritMichal Jastrzebski (inc0) proposed openstack/oslo.versionedobjects: Fixes for heat implementation  https://review.openstack.org/15483509:30
*** takedakn has quit IRC09:30
*** salv-orlando has quit IRC09:32
*** salv-orlando has joined #openstack-oslo09:32
*** salv-orlando has quit IRC09:35
*** salv-orlando has joined #openstack-oslo09:36
*** jecarey_ has quit IRC09:37
*** salv-orlando has quit IRC09:45
*** salv-orlando has joined #openstack-oslo09:46
*** ihrachyshka has joined #openstack-oslo09:49
*** salv-orl_ has joined #openstack-oslo09:49
*** salv-orl_ has quit IRC09:51
*** salv-orlando has quit IRC09:51
*** salv-orlando has joined #openstack-oslo09:51
openstackgerritMarian Horban proposed openstack/oslo-incubator: Using of waitpid system call was optimized  https://review.openstack.org/15634509:53
*** salv-orlando has quit IRC09:56
*** salv-orlando has joined #openstack-oslo09:56
*** exploreshaifali has joined #openstack-oslo10:03
*** yamahata has quit IRC10:15
*** takedakn has joined #openstack-oslo10:24
*** takedakn has quit IRC10:32
*** takedakn has joined #openstack-oslo10:34
*** takedakn has quit IRC10:56
*** kbyrne has quit IRC11:09
*** exploreshaifali has quit IRC11:11
*** kbyrne has joined #openstack-oslo11:13
*** cdent has joined #openstack-oslo11:13
*** e0ne has joined #openstack-oslo11:14
*** dims__ has joined #openstack-oslo11:19
*** amotoki has quit IRC11:20
*** exploreshaifali has joined #openstack-oslo11:24
*** exploreshaifali has quit IRC11:28
*** e0ne is now known as e0ne_11:40
dims__ihrachyshka: "Reach out, fix, help. Not complain." - golden words!11:42
*** e0ne_ has quit IRC11:50
*** takedakn has joined #openstack-oslo11:53
*** e0ne has joined #openstack-oslo12:18
*** salv-orlando has quit IRC12:20
*** takedakn has quit IRC12:24
*** amotoki has joined #openstack-oslo12:37
*** e0ne is now known as e0ne_13:05
*** jaypipes has joined #openstack-oslo13:05
*** himangi has joined #openstack-oslo13:09
*** e0ne_ is now known as e0ne13:09
*** rushiagr is now known as rushiagr_away13:09
*** kgiusti has joined #openstack-oslo13:15
*** zigo has quit IRC13:18
*** zigo has joined #openstack-oslo13:20
*** e0ne_ has joined #openstack-oslo13:24
*** salv-orlando has joined #openstack-oslo13:25
*** e0ne has quit IRC13:27
*** e0ne has joined #openstack-oslo13:30
*** e0ne_ has quit IRC13:33
*** gordc has joined #openstack-oslo13:37
*** trown has quit IRC13:49
*** vigneshvar has quit IRC13:49
*** himangi has quit IRC13:50
*** trown has joined #openstack-oslo13:50
*** jecarey has joined #openstack-oslo13:51
*** rluediger has joined #openstack-oslo13:59
*** rluediger has quit IRC14:01
*** jecarey has quit IRC14:03
*** exploreshaifali has joined #openstack-oslo14:04
*** rushiagr_away is now known as rushiagr14:07
*** dims__ has quit IRC14:15
*** dims__ has joined #openstack-oslo14:20
*** dims__ has quit IRC14:20
*** dims__ has joined #openstack-oslo14:21
*** dims__ has quit IRC14:25
*** amrith is now known as _amrith_14:29
*** mriedem has joined #openstack-oslo14:31
*** Guest37356 has joined #openstack-oslo14:35
*** jgrimm is now known as zz_jgrimm14:38
*** Guest37356 is now known as dims__14:49
*** ajo has quit IRC14:51
*** ajo has joined #openstack-oslo14:51
dansmithinc0: replied to your comments14:52
inc0yeah, I saw it, removing -1 :)14:53
dansmithheh, okay14:53
inc0also, I've added this patch as requirements to my14:53
inc0mine*14:53
dansmithinc0: okay, on the timezone thing, what you have looks fine, but since it's part of the api:14:54
* dansmith thinks14:55
dansmithwhat you have here lets me store a tz-aware or a tz-unaware datetime in the field if ensure_tzinfo=False14:56
dansmithwhich means that you could get two Foo objects, one with an aware datetime and one without, and then be unable to compare them14:56
inc0yeah, 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
dansmithI 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 negative14:57
inc0I don't see point of removing tzinfo if its there14:57
dansmithit's just to try to ensure consistency of what you have stored, since you can't compare aware and non-aware datetimes14:58
inc0I don't think its good eighter...its deleting data from object14:58
dansmithwell, you could just raise an exception instead of stripping it14:58
dansmithwhat I meant is, ensure that the awareness matches the flag14:59
inc0that would be better14:59
inc0allright, I'll do that14:59
*** salv-orlando has quit IRC15:00
dansmithinc0: 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
inc0dansmith, sounds good15:01
inc0dims__, I've started from heat15:01
inc0but now I'm helping guys from cinder with implementation15:01
inc0and other to get specs for L going;) like glance, neutron15:01
*** salv-orlando has joined #openstack-oslo15:02
dims__inc0: nice15:03
*** jecarey_ has joined #openstack-oslo15:03
*** mtanino has joined #openstack-oslo15:03
*** daniel3_ has joined #openstack-oslo15: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 chance15:05
inc0I 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 patch15:06
dims__haha, thanks15:06
dims__inc0: this one right? https://review.openstack.org/#/c/154835/15:06
inc0yup15:07
*** cdent_ has joined #openstack-oslo15:09
*** cdent has quit IRC15:09
*** cdent_ is now known as cdent15:09
inc0dansmith, on the closer look, I'll go for removing tzinfo, because timeutils injects it anyway;)15:15
inc0so by this flag I'll just ensure both ends are eighter tzinfo aware, or not, wile inside it will always be tzinfo aware15:15
*** sigmavirus24_awa is now known as sigmavirus2415:16
*** _amrith_ is now known as amrith15:18
*** e0ne is now known as e0ne_15:18
*** e0ne_ is now known as e0ne15:20
*** zz_jgrimm is now known as jgrimm15:22
*** himangi has joined #openstack-oslo15:24
*** stevemar has joined #openstack-oslo15:24
openstackgerritMichal Jastrzebski (inc0) proposed openstack/oslo.versionedobjects: Fixes for heat implementation  https://review.openstack.org/15483515:27
dansmithinc0: heh, okay, will look in a sec15:27
dhellmanngood morning, all15:27
inc0thanks15:30
inc0hello dhellmann15:30
dhellmanndims__: 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 release15:30
dhellmannhi, inc015:30
dhellmanndims__: that's oslo.policy, sorry, wasn't clear15:31
dims__dhellmann: in an hour, yes. will do15:31
dhellmanndims__: great, thanks!15:31
*** devlaps has joined #openstack-oslo15:32
*** exploreshaifali has quit IRC15:32
inc0dims__,  https://review.openstack.org/#/c/156397/ I think we should explicitly say that its for tests15:34
inc0hmf...maybe just file naming?15:35
inc0like test_utils.py or fakes.py? what do you think?15:37
openstackgerritMerged openstack/oslo.versionedobjects: Disable some unstable tests until they are generalized  https://review.openstack.org/15666015:37
dims__inc0: we have used fixture.py in other oslo projects15:43
dims__dims@dims-mac:~/openstack/oslo$ find . -name fixture.py15:43
dims__./oslo.config/oslo/config/fixture.py15:43
dims__./oslo.config/oslo_config/fixture.py15:43
dims__./oslo.context/oslo_context/fixture.py15:43
dims__./oslo.i18n/oslo/i18n/fixture.py15:43
dims__./oslo.i18n/oslo_i18n/fixture.py15:43
dims__./oslo.utils/oslo_utils/fixture.py15:43
*** zzzeek has joined #openstack-oslo15:43
inc0than maybe stick to this name?15:43
dims__but i am flexible, dont want to mandate15:43
inc0well, I'm ok with fixture, especially if its present in rest of oslo15:44
inc0my point is, testing alltogether has its own nomenclature, and I'd like to stick to it when we produce something test related15:44
inc0(but I'm not stubborn about it)15:45
openstackgerritMerged openstack/oslo.versionedobjects: Generalize the indirection_api interface  https://review.openstack.org/15639415:46
openstackgerritMerged openstack/oslo.versionedobjects: Remove Nova objects module registration code  https://review.openstack.org/15639515:46
openstackgerritMerged openstack/oslo.versionedobjects: Add conditional object registration  https://review.openstack.org/15639615:46
openstackgerritMerged openstack/oslo.versionedobjects: Generalize remote testing infrastructure  https://review.openstack.org/15639715:46
dims__inc0: i hate to suggest renaming stuff in the middle of a patch series as well :)15:47
inc0yeah, we can do it as another patch, works for me15:47
inc0this is just my idea of possible improvement, not mistake by itself15:48
*** hemnafk is now known as hemna15:49
*** jecarey_ is now known as jecarey15:49
*** jaosorior has joined #openstack-oslo15:53
*** yamahata has joined #openstack-oslo15:58
*** viktors is now known as viktors|afk16:02
openstackgerritDoug Hellmann proposed openstack/oslo.policy: Clean up configuration option management  https://review.openstack.org/15704416:02
inc0dims__, would you do the honors?:) https://review.openstack.org/#/c/154835/16:06
*** belliott has left #openstack-oslo16:08
*** inc0 has quit IRC16:09
*** harlowja_at_home has joined #openstack-oslo16:14
openstackgerritSteve Martinelli proposed openstack/oslo-incubator: Remove policy from oslo-incubator  https://review.openstack.org/15281216:16
openstackgerritSteve Martinelli proposed openstack/oslo-incubator: Remove policy from oslo-incubator  https://review.openstack.org/15281216:16
openstackgerritSteve Martinelli proposed openstack/oslo-incubator: Prevent update.py from updating policy  https://review.openstack.org/15281316:17
*** tsekiyama has joined #openstack-oslo16:18
*** pradk has joined #openstack-oslo16:24
stevemardims__, thanksfor reviewing16:27
*** david-lyle_afk is now known as david-lyle16:28
dims__stevemar: waiting for one more review (jobs still running)16:34
*** harlowja_at_home has quit IRC16:43
haypodo you know if eventlet supports reentrant lock?16:43
*** noelbk has joined #openstack-oslo16:46
ihrachyshkahaypo, 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 thread16:46
haypoihrachyshka: it's writing an article about dead locks in threads, asyncio and eventlet ;)16:47
openstackgerritMerged openstack/oslo.versionedobjects: Generalize object hash-based change detection  https://review.openstack.org/15639816:53
openstackgerritMerged openstack/oslo.versionedobjects: Generalize object dependency change detection  https://review.openstack.org/15639916:53
openstackgerritMerged openstack/oslo.versionedobjects: Add a test to ensure subclassibility of the object registry  https://review.openstack.org/15666216:58
openstackgerritMerged openstack/oslo.policy: Create the temporary files needed for tests  https://review.openstack.org/15681117:03
openstackgerritMerged openstack/oslo.policy: Change default set of tox environments  https://review.openstack.org/15681217:03
openstackgerritMerged openstack/oslo.versionedobjects: Fixes for heat implementation  https://review.openstack.org/15483517:09
openstackgerritMerged openstack/oslo.policy: Fix i18n imports  https://review.openstack.org/15681317:10
*** amotoki has quit IRC17:10
openstackgerritMerged openstack/oslo.policy: Update comments about tox configuration  https://review.openstack.org/15683617:10
*** i159 has quit IRC17:11
dansmithdims__: hey, you merged all my patches!17:15
*** daniel3_ has quit IRC17:17
*** daniel3_ has joined #openstack-oslo17:18
*** ihrachyshka has quit IRC17:18
*** daniel3_ has quit IRC17:18
*** daniel3_ has joined #openstack-oslo17:18
*** e0ne is now known as e0ne_17:22
*** e0ne_ is now known as e0ne17:24
openstackgerritBen Nemec proposed openstack/oslo.config: Log a warning when deprecated opts are used  https://review.openstack.org/14802017:27
*** e0ne has quit IRC17:31
*** dulek has quit IRC17:32
*** bknudson has joined #openstack-oslo17:33
*** dulek has joined #openstack-oslo17:33
dims__dansmith: is that good or bad? :)17:34
dansmithdims__: good :)17:34
dhellmannbnemec: 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.py17:35
bnemecdhellmann: I would be fine with it living there, but I think it needs cross-project buy-in.17:35
dansmithdims__: we have a fixtures thing already, maybe they should be merged17:36
dims__dansmith: ok, i'll take a stab17:36
bnemecdhellmann: 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
dansmithdims__: I'm proposing more changes to checks, so I'll slap a merge after I'm done17:36
dhellmannbnemec: yeah, that makes sense, I just wanted to mention it before I forgot17:36
dansmithdims__: or I can let you do it17:36
dhellmannbnemec: yeah, we don't have a good place for docs like this17:36
dims__dansmith: cool, not in a hurry17:36
*** daniel3_ has quit IRC17:36
dansmithdims__: okay17:36
*** daniel3_ has joined #openstack-oslo17:37
*** harlowja_away is now known as harlowja_17:42
*** exploreshaifali has joined #openstack-oslo17:43
openstackgerritDoug Hellmann proposed openstack/oslo.policy: Clean up configuration option management  https://review.openstack.org/15704417: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 sure17:57
harlowja_* https://github.com/eventlet/eventlet/blob/master/eventlet/green/threading.py#L1117:58
harlowja_so that prevents the eventlet sempahore (which replaces the threading lock) from being acquired twice by the same greenthread17:58
dhellmannbnemec: 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 IRC18:09
*** rushiagr is now known as rushiagr_away18:10
bnemecdhellmann: 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 IRC18:12
dhellmannbnemec: yeah, it also gives us the "die on a deprecated option" behavior that some operators like18:12
dhellmannbnemec: comment left -- I just wanted to make sure there wasn't a circular dependency reason or something18:13
bnemecdhellmann: Oh, also we have no incubator code in oslo.config today.18:13
*** palendae has left #openstack-oslo18:13
dhellmannthat's not an issue by itself18:14
bnemecI guess we have the cross-test script.18:14
bnemecYeah, I'll add versionutils.18:14
dhellmannthere might be a circular dependency issue, though, since that module uses oslo_config18:14
dhellmannwell, hang on18:14
dhellmannbnemec: 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 out18:16
bnemecOh, 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
bnemecAh, not update.py.  Just openstack-common.conf.  That's an easier fix. :-)18:26
openstackgerritMerged openstack/oslo.db: Imported Translations from Transifex  https://review.openstack.org/15689018:29
openstackgerritMerged openstack/oslo.config: Better check for integer range to account for 0  https://review.openstack.org/15194018:34
openstackgerritBen Nemec proposed openstack/oslo.config: Log a warning when deprecated opts are used  https://review.openstack.org/14802018:41
bnemecdhellmann: Actually there is a circular dependency with versionutils, but it's okay because we don't need it at import time anyway.18:41
dansmithdims__: so our existing fixtures thing (obj_fixtures) is in tests/ and it's things the objects tests need18:43
dansmithdims__: just to be clear, you're suggesting having a fixture.py outside the tests directory, just s/checks/fixtures/ right?18:43
dims__dansmith: yep18:43
dims__these are fixtures to be used in other projects18:44
dansmithdims__: okay18:44
openstackgerritJoshua Harlow proposed openstack/taskflow: Tweaks to atom documentation  https://review.openstack.org/15684418:47
*** cdent has quit IRC18:52
dims__dansmith: checks.py has mock, but mock is only in test-requirements18:54
dansmithdims__: hmm, is it legit for me to add something like mock to requirements?18:55
dims__trying to check other fixtures18:55
dims__dansmith: only one we do that so far is oslotest18:56
dansmithdims__: hmm18:56
*** achanda has joined #openstack-oslo18:56
*** e0ne has joined #openstack-oslo18:57
*** e0ne is now known as e0ne_18:57
*** e0ne_ is now known as e0ne18: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-oslo19:03
*** pradk has quit IRC19:07
dhellmannbnemec: with the circular dep we can't have the 2 libraries depending on each other19:08
dhellmanndims__: what do we do with other libs fixtures and the fixtures library19:09
dims__dhellmann: can't find any other fixture.py that needs mock19:09
dhellmanndims__: but they would all use the "fixtures" library, right?19:10
dhellmanndims__: https://pypi.python.org/pypi/fixtures19:10
dhellmanndims__: oslo.concurrency depends on fixtures directly19:12
dims__dhellmann: ah19:13
*** achanda has quit IRC19:14
dhellmannI think since this is part of the public API of this library, the dependency can go into requirements.txt19:14
dims__dansmith: checks.py imports fixtures, so we follow the same pattern as in oslo.concurrency19:14
*** bknudson has quit IRC19:14
dims__dhellmann: ack19:14
dhellmannif 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 well19:14
dhellmanndims__: how about writing this up as a policy for our specs repo?19:14
dansmithdims__: so fixtures and mock need to go in requirements, right?19:15
dims__dansmith: y19:15
dhellmanndansmith: yeah19:15
dims__dhellmann: is there another spec review like this i can pattern it on? since this is not a code related thing19:16
dansmithokay cool19:17
dhellmanndims__: all of the ones in the policy subdir19:17
dims__dhellmann: ack will look/submit19:17
dhellmanndims__: I just approved a bunch yesterday that had been up for review for a while without any -119:17
openstackgerritJoshua Harlow proposed openstack/debtcollector: By default mutate the docstring(s) for deprecations  https://review.openstack.org/15598819:19
openstackgerritJoshua Harlow proposed openstack/debtcollector: By default mutate the docstring(s) for deprecations  https://review.openstack.org/15598819:20
*** achanda has joined #openstack-oslo19:21
openstackgerritDavanum Srinivas (dims) proposed openstack/oslo.versionedobjects: Cleanup comment and H803 hacking  https://review.openstack.org/15711419:25
*** pradk has joined #openstack-oslo19:25
openstackgerritJoshua Harlow proposed openstack/debtcollector: By default mutate the docstring(s) for deprecations  https://review.openstack.org/15598819:27
*** sreshetnyak has quit IRC19:31
openstackgerritMerged openstack/oslo.policy: Clean up configuration option management  https://review.openstack.org/15704419:39
*** vigneshvar has quit IRC19:45
openstackgerritJoshua Harlow proposed openstack/debtcollector: By default mutate the docstring(s) for deprecations  https://review.openstack.org/15598819:46
*** crc32 has joined #openstack-oslo19:48
openstackgerritSridhar Gaddam proposed openstack/oslo.utils: Utility API to generate EUI-64 IPv6 address  https://review.openstack.org/13777419:56
*** bknudson has joined #openstack-oslo19:59
*** andreykurilin_ has joined #openstack-oslo20:00
*** vigneshvar has joined #openstack-oslo20:02
openstackgerritDan Smith proposed openstack/oslo.versionedobjects: Generalize compatibility testing  https://review.openstack.org/15712520:04
openstackgerritDan Smith proposed openstack/oslo.versionedobjects: Generalize the object relationships test  https://review.openstack.org/15712620:04
openstackgerritDan Smith proposed openstack/oslo.versionedobjects: Rename checks to fixture and update requirements  https://review.openstack.org/15712720:04
dhellmannbnemec: are you around to discuss the circular dep issue? I'm stumped.20:09
*** achanda has quit IRC20:09
bnemecdhellmann: 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 IRC20:13
*** andreykurilin_ has quit IRC20:15
*** achanda has joined #openstack-oslo20:17
openstackgerritDavanum Srinivas (dims) proposed openstack/oslo-specs: Policy for specifying external library in requirements.txt  https://review.openstack.org/15713520:22
bnemecdhellmann: Okay, crisis averted for now. :-)20:26
*** devlaps has quit IRC20:27
dhellmannbnemec: so I have 3 alternatives for what we do with versionutils, and none seems great20:30
dhellmann1. move it to oslo.utils, introducing a dependency between oslo.utils and oslo.config20:30
dhellmann2. move it to its own library20:30
dhellmann3. leave it in the incubator indefinitely20:30
dhellmannonly option 3 lets us avoid a circular dependency between libraries :-/20:31
bnemecdhellmann: Hmm, I'm not coming up with any terrific options either.20:34
dhellmannI guess alternative 4 is to put the code in oslo.config, but ew.20:35
bnemecWe 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
dhellmannright20:35
dhellmannthe only options I'm coming up with for decoupling them involve complex callback registry schemes20:36
dhellmannbnemec: of all the times I think you would want to exit the app on a deprecation warning, a config option seems the most likely20:38
bnemecdhellmann: Maybe we could make the oslo.config dep in versionutils optional?  Default to non-fatal if it's not present?20:38
dhellmannbnemec: hmm, or the other way around. If oslo.config can't import the versionutils code, it just uses LOG.warn()20:39
bnemecYeah, that could also work.20:39
harlowja_versionutils can't go in oslo.log?20:40
dhellmannharlowja_: oslo.config and oslo.log can't have a circular dependency either20:40
openstackgerritEric Brown proposed openstack/oslo.vmware: Resolve warnings from Bandit  https://review.openstack.org/15688920:40
harlowja_hmmm20:40
bnemecharlowja_: The problem is I just changed https://review.openstack.org/#/c/148020/ to use versionutils.20:40
*** yamahata has joined #openstack-oslo20:40
* dhellmann suggested it20:40
harlowja_hmmmmssss20:40
bnemecThat change has a bad record with reviewer suggestions.  They all turn out to be deep, deep rabbit holes. :-)20:41
dhellmannheh20:41
dhellmannthe 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, ha20:42
dhellmannthe value would default to LOG.warn, but an app could pass in oslo.versionutils.whatever-that-function-si20:42
dhellmannbut that feels a bit heavy-handed for one function20:42
harlowja_dhellmann agreed, would work but does feel sorta odd20:43
dhellmannit also feels pretty odd to put an "I'm going to exit" call in a library20:43
dhellmannso maybe the right answer really is to just leave versionutils in the incubator indefinitely20:43
bnemecdhellmann: It doesn't exit though.  It just raises an exception.20:44
dhellmannalthough that's sort of a technicality20:44
harlowja_bnemec does anyone catch it :-P20:44
dhellmannoh, true, I guess I just assumed it wasn't being caught20:44
bnemecI'm betting there are lots of except Exceptions that would catch it. :-)20:44
*** devlaps has joined #openstack-oslo20:44
dhellmanntrue20:44
bnemecAnd it's off by default.  The user has to explicitly ask for that behavior.20:45
dhellmannyeah20:46
dhellmannbnemec: 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
bnemecdhellmann: 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 :-P20:50
dhellmannharlowja_: it would have been technically easier to release the incubator as one big library, except maintenance would have been a nightmare20:51
harlowja_:)20:51
* dhellmann is startled to see blue sky and snow flurries through the same window20:52
openstackgerritBen Nemec proposed openstack/oslo.config: Log a warning when deprecated opts are used  https://review.openstack.org/14802020:53
dhellmannbnemec: see my other comment on PS6 about the implementation of _check_deprecated() though, with the names20:54
bnemecAh, yeah.  I should have fixed that first.20:54
bnemecWould have saved me having to trick Gerrit into letting push a duplicate commit. :-)20:54
dhellmann:-)20:55
dhellmanndims__, 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-oslo21:02
* dhellmann throws caution to the wind and releases oslo.policy 0.1.021:03
*** mriedem has quit IRC21:03
dims__dhellmann: yay!21:04
dhellmannhmm, our release notes script doesn't handle this case21:07
dhellmanndims__: 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: yes21:09
dhellmannok, we'll do the same thing here then21:09
dims__we did that for oslo.log21:09
dhellmannno announcement email21:09
dhellmannright, that's the one21:09
dhellmannwe 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 now21:10
dims__someone eager to try it out,21:11
dhellmannI thought I already closed that as invalid21:11
openstackgerritBen Nemec proposed openstack/oslo.config: Log a warning when deprecated opts are used  https://review.openstack.org/14802021:11
dims__ah ok dhellmann21:11
dhellmanndims__: I waffled, so I had to check, but yes, I did.21:12
harlowja_u had a waffle21:13
dhellmannharlowja_: now I'm hungry21:14
harlowja_me too21:14
harlowja_lol21:14
dhellmannbnemec: isn't an empty group name the same as DEFAULT?21:15
stevemar\o/21:16
*** amrith is now known as _amrith_21:16
bnemecdhellmann: I don't think so: https://github.com/openstack/oslo.config/blob/master/oslo_config/cfg.py#L153821:17
dhellmannbnemec: I sure hope the default is None, then :-)21:18
bnemecdhellmann: Yeah, I was tempted to try it, but decided that was a rabbit hole too far. ;-)21:19
dougwighi oslo.  is this perhaps fallout from the namespace rename?21:20
dougwighttps://www.irccloud.com/pastebin/hb74zcUv21:20
dhellmanndougwig: maybe? is that code in octavia?21:22
openstackgerritMerged openstack/oslo-specs: Add revision history sections  https://review.openstack.org/15261821:22
dhellmanndougwig: it looks like maybe someone went a little too far with a sed conversion of oslo.config to oslo_config21:23
dougwigyes.  i've never touched that except via oslo-incubator/update.py, and it seems that oslo_config got text replaced to oslo.config somehow21:23
dhellmannoh, hmm21:23
dhellmannmaybe we need to fix up update.py then21:23
openstackgerritMerged openstack/oslo-specs: Add adoption timeline to incubator policy  https://review.openstack.org/15368221:24
dhellmanndougwig: I can reproduce that when I run update.py. I'm looking at it.21:28
dhellmanndougwig: have you already filed a bug?21:28
*** andreykurilin_ has joined #openstack-oslo21:32
openstackgerritDan Krause proposed openstack/taskflow: add get_flow_details and get_atom_details to all backends  https://review.openstack.org/15715321:34
openstackgerritEric Brown proposed openstack/oslo.vmware: Resolve warnings from Bandit  https://review.openstack.org/15688921:34
zzzeekbnemec: 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
bnemeczzzeek: /join openstack-cinder?21:38
bnemecOr open a bug.  Or send something to the list tagged [cinder]21:38
zzzeekbnemec: OK so, I should remove this patch then, and instead have it raise an error21:39
zzzeek?21:39
zzzeekbsically youre saying , there’s no such class of behavior such as, “this is less than ideal but we can correct for it"21:39
dougwigdhellmann: not yet.  want one?21:39
bnemeczzzeek: 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
dhellmanndougwig: yeah, if you have a minute. I think I have a fix for you but need to run some tests.21:40
bnemecNot when we can open a bug against cinder and say "fix this".21:40
dougwigok, standby, meatspace intruding.21:40
zzzeekbnemec: what if we are correcting for many things that affect performance negatively?21:40
zzzeekbnemec: we just keep stuffing corrections in, hiding them, and thats that ?21:40
bnemeczzzeek: Tell the people who can fix it, not the operators.21:40
bnemecThat's all I'm saying.21:41
zzzeekbnemec: 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 office21:41
zzzeekbnemec: also what about warnings.warn() ?21:41
zzzeekbnemec: 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 corrected21:41
zzzeekthose should all be…hidden ?21:41
bnemecI'd be fine with warnings.  They don't go to the logs by default.21:41
zzzeekbnemec: OK so can i cahgne this to warnings.warn() ?21:41
zzzeekbnemec: because it really isnt INFO and it really hsouldnt be hidden21:42
bnemeczzzeek: That's why I'm suggesting debug.  That's the developer-oriented log level.21:42
bnemecBut yeah, I'm fine with the warnings module.21:42
bnemecThat seems developer oriented as well.21:42
zzzeekbnemec: who ever reads DEBUG, and if they did, how do they tell things that are “normal” from things that are errors?21:42
bnemecsdague: ^Just FYI, it seems we could use some more clarification on the target audience in the logging spec.21:45
bnemecWe seem to have a difference of interpretation.21:46
sdaguecan someone summarize the question?21:47
zzzeekbnemec: 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
zzzeeksdague: “the code is doing a bad thing that we can recover from, but the develoipers should fix it"21:47
zzzeeksdague: how to log ?21:47
bnemecFull discussion here: https://review.openstack.org/#/c/156725/2/oslo_db/sqlalchemy/session.py21:47
sdaguewhich developers are the target?21:48
zzzeeksdague: poeple who commit to cinder, for example21:48
zzzeeksdague: someone who would make it so that they call engine.dispose() before launching a child process21:48
zzzeekwell, wihtin the child proc.21:48
zzzeekwarnings.warn() might be the most appropriate place21:49
sdagueyou are throwing an exception already though right? Isn't that the "don't do this thing" indication to developers21:49
zzzeekbut warnings.warn() has the major issue that you cant put dynamic inforamtion in it without the chance of leaking memory21:49
dougwigdhellmann: lol21:49
zzzeeksdague: no, we can recover from it and continue.  i can make it just throw instead, but that would be more disruptive21:49
zzzeekbnemec: 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 general21:50
bnemeczzzeek: Yeah, it's a good discussion.21:51
sdaguezzzeek: yeh, so here's the thing we've discovered. developers basically don't read the logs.21:51
sdagueso it's not actually developer communication21:51
zzzeeksdague: well this is the thing with warnings, people complain about them, so they get noticed21:51
sdaguewhat it actually turns into is "dear god, my cloud is busted"21:51
zzzeeksdague: sure21:51
zzzeeksdague: no middle ground huh21:51
bnemecAlso, I actually think this behavior is broken anyway.  Who knows what other file descriptors they've accidentally passed to child processes?21:52
bnemecMaybe we could add a flag to make this fatal in the gate?21:52
zzzeekbnemec: OK…how would they guard against that across-the-board ?21:52
bnemecThat would get noticed by devs. :-)21:52
sdaguebnemec: yeh, that's why I was wondering about the raise21:52
dougwigdhellmann: https://bugs.launchpad.net/oslo-incubator/+bug/142336121:53
openstackLaunchpad bug 1423361 in oslo-incubator "incubator update.py converts oslo_config in cache module method name" [Undecided,New]21:53
bnemeczzzeek: Fork their workers before doing things like opening files and db connections?21:53
sdaguebecause that seems to be the thing that we want to do, figure out how to signal the right audience to address it21:53
zzzeekthis 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 proc21:53
zzzeekbnemec: OK….but this seems to be some kind of ad-hoc process spawning thing that oslo.concurrency is doing21:53
zzzeekbnemec: it looks to be designed to work this way21:53
zzzeekbnemec: these arent long running processes21:54
bnemeczzzeek: Well, processutils was more intended for doing things like running iptables from a service.21:54
zzzeekbnemec: OK so, I can make this jsut raise.  want me to do that?21:54
zzzeekbnemec: then cinder has to fix it on their end21:54
zzzeekbnemec: but that means, new oslo.db comes out, every app doing this somehwere breaks21:55
bnemeczzzeek: Yeah, exactly.21:55
zzzeekdhellmann: are you watching this convo21:55
bnemecIf we want this to be fatal, I think it has to be _only_ in the gate.21:55
bnemecLike the fatal deprecations thing in versionutils.21:55
zzzeekbnemec: OK and if we’re not in the gate…then what ?  silent recovery ?21:55
* bnemec brings the conversation full circle ;-)21:56
sdaguebnemec: so, honestly, I personally think having the library not try to recover is the right philosophical thing to do21:56
sdaguebecause I actually see issues like that pop up a bunch21:56
sdaguelike the embedded retry in execute21:56
bnemecI actually agree.  This will also break horribly under py3.21:56
*** e0ne has quit IRC21:56
bnemecUnfortunately in py2 it's an intermittent failure, apparently.21:57
zzzeekbnemec: b.c. just any open FD will raise immediately ?21:57
zzzeekbnemec: if i have a proces, and i open an FD, then say fork(), it breaks immediately in py3?21:57
bnemeczzzeek: Because open fds get closed by default on forks.21:57
bnemecpy3 sets O_CLOEXEC on file descriptors21:57
zzzeekbnemec: this recovery I have will work fine in py3k21:57
zzzeekbnemec: the descriptor is never touched21:57
bnemeczzzeek: Right, because you detect the problem and reopen it?21:58
zzzeekbnemec: 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 immediately21:59
sdaguezzzeek: 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
zzzeeksdague: OK what log level22:00
sdagueif you get that explicit, WARN seems fine to me22:00
zzzeekDOH @22:00
zzzeekheh22:00
zzzeeksdague: well thats waht the whole argument was about :)22:00
* dims__ gets some popcorn :)22:00
* dhellmann reads scrollback22:00
* zzzeek has no opinion! does not care! 22:01
sdaguezzzeek: right, but in order to do that, you need to really give the ops a straight line to the right dev place to complain22:01
zzzeeksdague: OK do you want to just make a comment on the review then, https://review.openstack.org/#/c/156725/2/oslo_db/sqlalchemy/session.py22:01
sdaguebecause 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 issues22:01
zzzeekits between you and bnemec what you want to do, he was opposed to WARN22:01
bnemecYeah, we did that before and got a lot of complaints.22:02
sdaguebnemec: where did you used to do it?22:02
bnemec"Why are you warning me about this thing I can't fix?"22:02
sdagueoh, yeh, that, agreed22:02
zzzeekyou know what woudl be great.   we decide what we want here.  then we *update that spec* to be clear about this22:02
sdagueif you are going to flag it that high you need to provide a path to fixing it22:02
bnemecThat was the traditional sql mode thing, and we basically just turned it on by default and prayed.22:03
bnemecWe got less complaints about that than the warning, oddly enough.22:03
bnemecApparently we were already pretty traditional-safe.22:03
sdaguebnemec: I know I bitched about that one :)22:03
* sdague waiting for dhellmann to weigh in 22:04
bnemecHeh22:04
* dhellmann has almost caught up22:04
* dhellmann wonders why walking away at 4:55 always results in so much scrollback22:04
haypodhellmann, 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.rst22:04
hayposorry, each version of the spec is longer than the previous spec!22:04
dhellmannsdague, bnemec, zzzeek : so the options are log a message intended for devs, use warnings.warn, or throw an exception?22:05
haypothe most interesting change is a new section on "race conditions"22:05
zzzeekdhellmann: or just consider this particular thing as “OK”22:05
sdaguedhellmann: I think the options are22:05
dhellmannzzzeek: that's the "let the library recover" option?22:05
sdague1) don't recover from the error, put the burden on the caller22:05
zzzeekdhellmann: yes.22:06
haypoharlowja_: ah yes, monkey-patched threading.RLock may work in eventlet22:06
* dhellmann waits for sdague to finish enumerating22:06
sdague2) log something and recover22:06
sdagueif 2)22:06
sdaguethen 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 audience22:07
sdagueWARN "Invalidating connection detected as shared to new... "22:07
dhellmannwhat about logging and not recovering? then the log message will be in the job that fails22:07
dhellmanner, in the output for the job that fails22:07
sdagueis mostly going to just confuse people reading the message22:07
bnemecAlso, it sounds like this happens every run, but only intermittently causes failures.22:08
dhellmannyeah, 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 separately22:08
zzzeekbnemec: well for sure, it causes random race conditions22:08
zzzeekbnemec: but yes its on every run if certain options in cinder are turned on22:09
zzzeekbnemec: that make it go off and use oslo.concurrency when it starts up22:09
bnemecYeah, so we need all or nothing.  Either recover, or fail immediately.22:09
zzzeekOK well, that gerrit has “the code”.   you all can make it do whatever :)22:09
dhellmannhow about adding just the log message for now and cutting a release so we can measure how often it happens in projects other than cinder22:09
zzzeekdhellmann: OK.  What level ? :)22:09
dhellmannthat way we can tell whether it's safe to make it fail immediately22:10
sdagueso I think that's actually the crux of the problem really22:10
dhellmannzzzeek: what level do we log at for developers in gate jobs?22:10
zzzeekdhellmann: shrugs ?22:10
dhellmanndebug, I would think22:10
sdaguebecause it's like "oh, this thing is wrong, it's your problem, except, I'll fix it for you"22:10
zzzeekdhellmann: OK !22:10
dhellmannI don't know if we actually turn the logging up that high22:10
zzzeekdhellmann: debug it is22:10
sdagueyes, we use DEBUG in the gate22:10
zzzeekdhellmann: any wording in the DEBUG that make it clear that “hey, you should probably see why this is happening” ?22:10
zzzeekdhellmann: like a prefix ?22:10
bnemecIt'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
zzzeekand …can this be added to the spec ??   to avoid 30 min IRC discussion ?22:11
dhellmannsdague: 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
dhellmannzzzeek: I believe the spec already says that developer messages should be logged at debug22:11
zzzeekdhellmann: OK but how do we distinguish “normal operations” DEBUG from “HEY!  this is BAD!” DEBUG ?22:11
zzzeekdhellmann: just need the lingo22:12
zzzeekdhellmann: if someoen wants to just pop it onto the gerrit here22:12
dhellmannzzzeek: 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
bnemecSort of serious suggestion: LOG.debug('WARNING: ...')22:12
dhellmannzzzeek: operators should not need to read debug messages22:12
zzzeekdhellmann: OK….but we recovered, so there wont be any race conditions22:12
zzzeekdhellmann: that is fine22:12
sdaguezzzeek: I'd actually prefix with "BUG: "22:12
zzzeekdhellmann: OK, so verbiage here is….ah “BUG:”, sure22:12
dhellmannzzzeek: I'm suggesting that you remove the fix and leave the race condition for now22:12
sdagueand I think it's a good point, and we should add to the spec22:12
zzzeekdhellmann: wow really.    UM, wow.  OK22:13
dhellmannso JUST log, and let's release a version of the lib that does that and see what we get in logstash22:13
sdagueI'll throw a patch up tomorrow22:13
dhellmannzzzeek: hold your horses22:13
zzzeekhey i have to pick up my wife, takes  20 minutes can you folks update me via gerrit / etc?22:13
dhellmannI suggest a 2 phase approach.22:13
bnemecCouldn't we fix the race and log the message to see what we get in logstash?22:13
dhellmannor we can all talk at once22:13
* zzzeek has to bbl, sorry22:13
dhellmannphase 1: log only the condition that is the problem, without fixing it22:13
dhellmannphase 2: based on how many apps we find with that error, decide whether to fix it by recovering or introduce an exception22:14
haypo"zzzeek: Because open fds get closed by default on forks." *_CLOEXEC only ask to close the fd on exec(), not at fork22:14
dhellmannif 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 that22:14
sdaguedhellmann: debug is not indexed in logstash, so that plan isn't going to work22:14
dhellmannok, well, then we'll have to log at info or something so we can measure it22:15
dhellmannwe can change it to debug as part of phase 222:15
sdaguethe gate is only one access pattern though22:15
dhellmanntrue22:15
sdagueso 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
dhellmannok, 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
sdaguemailing list thread? make sure to cc ptls of known effected projects?22:16
dhellmannwe don't know which projects are affected, though, that's the point22:17
dhellmannso far we only know about cinder22:17
bnemecCinder is the only known affected right now.22:17
dhellmannif it's only cinder, why is this even an oslo bug?22:17
dhellmannmy guess is that we'll find other projects22:18
dhellmanncan we find them through looking at code instead of running gate jobs?22:18
dims__dhellmann: i'd suspect neutron as well22:18
bnemecIt's not really an oslo bug, but it ends up looking like one.22:18
sdagueso if you expose on the ML list as a more general thread, maybe you'll get other people chiming in22:18
sdagueor paying attention, I could be overly optimistic there22:18
bnemecProbably :-)22:19
dhellmannsdague: yeah, I'm a bit pessimistic about that but it's worth a try22:19
sdaguebut that does seem like the cross dev community communication bus22:19
dhellmannI'd like to at least agree on an approach to put in that email,t hough22:19
dhellmannand frankly, I expect most people to want the faster horse instead of the car22:19
dhellmanni.e., recover quietly and leave me alone22:20
bnemecYeah, same thing with the monkey patching22:20
dims__dhellmann: my vote would be on "don't recover from the error, put the burden on the caller"22:20
sdaguedhellmann: maybe, it ends up hiding a ton of misuse issues22:20
dhellmannsdague: oh, I agree, I'm just making predictions22:20
haypozzzeek: "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
sdagueyeh, my inclinations is that libraries shouldn't do recovery like this22:20
dims__+122:21
dhellmannhaypo: yeah, the child process is reusing the open database connection22:21
dhellmannsdague: right, it doesn't tend to work out well doing it at that low a level22:21
haypodhellmann: how does the child process pick the file descriptor if it is now aware that it inherited unwanted file descriptors?22:21
dhellmannbnemec: can you start the mailing list thread? you have more background on this than I do22:21
bnemecOoh, what about a deprecation period for the current behavior?22:21
dhellmannhaypo: https://review.openstack.org/#/c/156725/2/oslo_db/sqlalchemy/session.py22:22
bnemecWe start by logging the warning, then next cycle or whenever we make this fail fast?22:22
dhellmannbnemec: I think we want the bug fixed this cycle, though22:22
dims__bnemec: 1 release of oslo.db, not one full cycle22:22
bnemecWe can do the current workaround during the deprecation period.22:22
bnemecdims__: I'd want a full cycle to give people a chance to find and fix all the problems.22:22
* bnemec hears crickets22:24
dhellmannI'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 change22:25
dims__they will change only when we break them22:25
dhellmannI 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 dhellmann22:25
bnemecdims__: Right, but oslo releases can happen within a couple of weeks.22:25
dims__bnemec: we can hold the line on oslo.db22:25
bnemecYeah, until we merge another critical bug fix. :-)22:26
dims__haha22:26
dhellmannI 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 IRC22:28
bnemecI 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
dhellmannbnemec: did you agree to start that email thread?22:29
dhellmannbnemec: 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 library22: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 production22:30
bnemecdhellmann: I guess so22:30
dhellmannwe could, but we don't *need* to22:30
*** achanda has quit IRC22:30
bnemecI'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, lol22:31
dims__:) harlowja_22:32
bnemecLogging.  Serious business.22:32
dims__haha bnemec22:32
harlowja_;)22:32
haypodhellmann: 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 module22:32
dhellmannbnemec: 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 IRC22:33
dhellmannhaypo: I'm not sure they are using subprocess(), but you'd have to ask zzzeek because he's the one that found the problem22:33
bnemecdhellmann: Okay, I'll just try to list the options we've discussed then.22:33
dhellmannbnemec: yeah22: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
dhellmannhaypo: this thread may have more detail: http://lists.openstack.org/pipermail/openstack-dev/2015-February/057097.html22:34
* harlowja_ goes back to being quiet (to much detail i dont know about, ha)22:35
harlowja_^ be quiet josh, ha22:35
dhellmannharlowja_: the app should be doing that, but isn't22:35
dhellmannthat's the actual bug22:35
haypodhellmann: i guess that cinder uses fork, not subprocess.Popen22:35
haypodhellmann: thanks for the links22:36
dhellmannhaypo: that may be the case22:36
bnemechaypo: Specifically this http://lists.openstack.org/pipermail/openstack-dev/2015-February/057184.html22:36
bnemecThe 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
openstackgerritDoug Hellmann proposed openstack/oslo-incubator: Fix the regex for turning project.lib back to oslo.lib  https://review.openstack.org/15717822:39
harlowja_^ all little fork 'helpers'22:40
*** noelbk has joined #openstack-oslo22:40
haypooh, i remember with sileht explained me that the Service class is based on fork() and does crazy things to workaround fork() issues22: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 IRC22:42
harlowja_maybe something idk (or some history there)22:42
haypoharlowja_: Service design is completly crazy :)22:43
harlowja_haypo no disagreement there :-P22:43
harlowja_some crazy history is highly likely, lol22:43
hayposince service.py is still in the incubator, it's not too late to fix it ;)22:44
dhellmannwell, 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
dhellmannharlowja_: quite possibly22:45
harlowja_although i do wonder how cinder is executing code before service.py does stuff22:45
dhellmannbnemec: 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-oslo22:46
harlowja_aw, sweet, we shoudl use this one22:46
harlowja_https://github.com/Gamocosm/minecraft-server_wrapper/blob/master/daemon.py#L6222:46
harlowja_and turn service.py into a minecraft server when its idle22:47
harlowja_lol22:47
harlowja_^ joke...22:47
haypoanyway, i contributed to https://bugs.launchpad.net/cinder/+bug/1417018 with my theory on fork22:48
openstackLaunchpad 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 IRC22:48
bnemecdhellmann: Yeah, if we could fix it in the service code that would be nice.22:49
bnemecEspecially since that hasn't graduated yet.22:49
dhellmannright22:49
dhellmannzzzeek doesn't say which process in cinder has the issue22:49
zzzeekdhellmann: it has to do with ceph22:50
zzzeekdhellmann: i had some log output in the email thread22:50
dhellmannzzzeek: 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#L6222:51
*** yamahata_ has quit IRC22:51
zzzeekdhellmann: 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-oslo22:51
harlowja_rootwrap stuff all goes through subprocess right?22:51
harlowja_*via execute()22:51
harlowja_*via process utils execute22:52
haypoharlowja_: the "CMD ..." log comes from execute() which uses subprocess.Popen with close_fds=True22:52
dhellmannzzzeek: which process was calling that rootwrap command, I mean22:52
zzzeekdhellmann: hard to say22:52
dhellmannI'm trying to understand which daemon it is, so I can look at that daemon's startup code22:52
zzzeekdhellmann: oh22:52
zzzeekdhellmann: well its cinder-volume22:53
zzzeekdhellmann: i have step-by-step in the bug report22:53
zzzeekto reproduce22:53
dhellmannzzzeek: the bug isn't linked from https://review.openstack.org/#/c/156725/2 is it in the email thread somewhere?22:54
zzzeekdhellmann: oh. hm22:56
dhellmannhmm, this looks like a fundamental issue with the way service works22:56
dhellmanncreate() takes the name of a manager class, and the instantiates it22:56
zzzeekdhellmann: its in one of my comments there its https://bugs.launchpad.net/cinder/+bug/141701822:56
dhellmannin __init__22:56
openstackLaunchpad 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
dhellmannzzzeek: next revision it would be good to add that to the commit message22:57
zzzeekdhellmann: yeah i didnt know the system to add bugs from a different project?22:57
zzzeekdhellmann: just full hyperlink ?22:57
dhellmannzzzeek: no, just "Closes-Bug: #xxx" or whatever22:57
zzzeekdhellmann: OK but that bug number is local to cinder, not oslo22:57
dhellmannlaunchpad bugs can be associated with more than one project22:58
zzzeekdhellmann: cross-project bug linking basically22:58
zzzeekdhellmann: OK22:58
zzzeekdhellmann: but..this is an oslo.db bug ? :)22:58
dhellmannzzzeek: reload the bug report, now it's associated with both cinder and oslo.db22:58
zzzeekdhellmann: yes.  so this is an oslo.db bug also?22:58
zzzeekdhellmann: menaing, “oslo.db does not guard against XYZ” ?22:59
haypozzzeek: the root cause should be investigated IMO22:59
dhellmannzzzeek: no, not really. I think the right thing to do here is fix the service code so we close file descriptors before forking22:59
zzzeekhaypo: the root cause is exactly known22:59
haypozzzeek: ah? what is it?22:59
dhellmannzzzeek: in general I don't like the libraries trying to recover from issues that came from elsewhere22:59
zzzeekhaypo: what I stated in the bug report22:59
*** openstackgerrit has quit IRC23:00
zzzeekhaypo: file descriptor is traveling to a new child process23:00
dhellmannzzzeek: I don't think it's that subprocess call, though.23:00
haypozzzeek: see my comment: i don't undestand your analysis23:00
*** openstackgerrit has joined #openstack-oslo23:00
zzzeekdhellmann: well if I make that call fail, then the bug goes away23:00
dhellmannsubprocess closes file descriptors23:00
zzzeekhaypo: why dont you run my step-by-step reproucition steps, pdb into it and let me know what you think ?23:00
zzzeekdhellmann: does it close FDs that are open by native -C code like MySQLdb ?23:01
zzzeekdhellmann: but actually i can reproduce it with pymysql also23:01
haypozzzeek: it's around midnight, i'm not supposed to work :) maybe tomorrow if you can help me to prepare the setup to reproduce the issue23:01
zzzeekdhellmann: bnemec tells me subproces only closes FDs in python 323:01
haypozzzeek: yes, close_fds=True closes all file descriptors23:01
zzzeekhaypo: the steps are all there, do you know how to run devstack ?23:01
haypozzzeek: sure, i have a devstack which is supposed to work :-)23:01
dhellmannzzzeek: http://git.openstack.org/cgit/openstack/oslo.concurrency/tree/oslo_concurrency/processutils.py#n21323:02
haypozzzeek: oslo.concurrency sets explicitly close_fds to True, so it also close FDs on Python 223:02
zzzeekhaypo: OK, so the bug does not exist, lets close it ! :)23:02
haypothat's why i'm asking if the issue doesn't come from fork23:02
zzzeekhaypo: whats the other way for a linux proces to make a child process besides fork ?23:03
haypozzzeek: Popen uses fork+exec. the Service class only uses fork23:03
dhellmannzzzeek: I suspect this is our real issue: http://git.openstack.org/cgit/openstack/oslo-incubator/tree/openstack/common/service.py#n30823:03
dhellmannbecause the service class is set up with a database connection before the launcher forks23:03
haypodhellmann: i love fork so much23:03
* dhellmann should be putting a fork into his dinner soon23:04
* haypo prefers spoons23:04
dhellmannzzzeek: for example, if cinder-volume starts multiple workers, they would *all* share that initial database connection23:04
* bnemec might be on a liquid diet after this discussion23:04
zzzeekdhellmann: 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, ha23:05
zzzeekif you all like, I can log the process id in oslo.concurrency there and prove its the same number in the error condition23:05
dhellmannzzzeek: yeah, I believe you, I'm just not sure I understand why it happens there23:05
zzzeekdhellmann: gonna say close_fds does not do waht we think it does in py2k or the flag is not set to True23:05
dhellmannthat would be interesting to see, too. Maybe cinder is passing false for that.23:06
haypodhellmann: "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
bnemecIt's not configurable.  The only time it's false is on windows.23:06
dhellmannzzzeek: how many workers does devstack create for cinder-volume with that configuration?23:06
haypodhellmann: that's completly insane. how is it possible that it worked before? :)23:06
zzzeekdhellmann: 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 child23:07
dhellmannhaypo: ah, I hadn't noticed that23: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 steps23:07
dhellmannoops, that was for bnemec ^^23:07
dhellmannzzzeek: do you have any reports of this problem in projects other than cinder?23:08
zzzeekdhellmann: nope23:08
*** pradk has quit IRC23:08
dhellmannok, that's something23:08
zzzeekdhellmann: however, the symptoms are extremely obtuse and distant from the acvtual problem23:09
zzzeekdhellmann: very occasional MySQL screwups23:09
haypodhellmann: a friend told me that after fork, the POSIX standard only allows a very low number of syscalls, something like read, write, exec23:09
dhellmannso 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
zzzeekdhellmann: 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 consistently23:09
dhellmannI'm not sure if I want to throw an error or not23:09
haypodhellmann: fork is only supposed to be used to execute fork, that's all :)23:09
haypooops23:09
haypodhellmann: 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 bit23:09
zzzeekharlowja_: and the two other folks who reported the issue, yup23:10
dhellmannhaypo: that's not true, processes fork() to daemonize themselves all the time23:10
harlowja_space rays could of hit them too zzzeek23:10
*** daniel3_ has quit IRC23:10
dhellmannzzzeek: 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
haypodhellmann: ah maybe. but instanciate the application after the fork would avoid many issues23:11
bnemecdhellmann: 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
dhellmannhaypo: 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
zzzeekdhellmann: 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
dhellmannbnemec: 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 more23: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 Z23:13
harlowja_option Z please, thx23:13
* zzzeek does the happy this-is-not-his-code dance23:13
dhellmannzzzeek: 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 issue23:13
* harlowja_ runs aawy23:13
zzzeekdhellmann: 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
dhellmannzzzeek: cool, like I said, just making sure the delay isn't annoying :-)23:14
zzzeekid encourgage everyone to jump on the bug report and do their worst23:14
haypoFYI I'm interested to help to redesign the service class. but it may be difficult to change it in stable branches23:14
hayposo zzzeek specific issue maybe need other changes in stable branches23:15
zzzeekhaypo: if other proejcts are forking and not disposing their engines and we decide not to correct for it in the pool, then yes23:15
haypowe may need a new service class which has a sane design, to not break applications using the old class23:15
haypozzzeek: i don't know what is the best place to workaround the fork issue, i don't know enough how SQLAchemy is used23:16
haypoi have to go, bye23:17
dhellmannit's time for me to head out, too23:18
bnemecGood night everyone.23:19
harlowja_i'm still around (now all lonely) :(23:23
*** noelbk has quit IRC23:24
*** noelbk has joined #openstack-oslo23:28
*** dims_ has joined #openstack-oslo23:49
*** andreykurilin_ has quit IRC23:50
*** dims__ has quit IRC23:51
*** vigneshvar has quit IRC23:51

Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!