openstackgerrit | A change was merged to openstack/taskflow: Split requirements into py2 and py3 files https://review.openstack.org/114641 | 00:11 |
---|---|---|
*** zzzeek has quit IRC | 00:13 | |
*** mtanino has quit IRC | 00:22 | |
openstackgerrit | Joshua Harlow proposed a change to openstack/taskflow: Add a futures type that can contain our future functionality https://review.openstack.org/116489 | 00:30 |
*** stevemar has joined #openstack-oslo | 00:42 | |
*** liusheng has quit IRC | 00:48 | |
*** liusheng has joined #openstack-oslo | 00:48 | |
*** liusheng has joined #openstack-oslo | 00:49 | |
*** GheRivero has quit IRC | 00:50 | |
*** GheRivero has joined #openstack-oslo | 00:51 | |
*** dims has joined #openstack-oslo | 00:59 | |
*** arnaud has quit IRC | 01:01 | |
*** dims has quit IRC | 01:03 | |
*** dims has joined #openstack-oslo | 01:04 | |
*** dims_ has joined #openstack-oslo | 01:06 | |
*** dims has quit IRC | 01:08 | |
openstackgerrit | Joshua Harlow proposed a change to openstack/taskflow: Use explicit WBE request state transitions https://review.openstack.org/114162 | 01:13 |
*** dims_ has quit IRC | 01:14 | |
*** dims has joined #openstack-oslo | 01:14 | |
openstackgerrit | Joshua Harlow proposed a change to openstack/taskflow: Tweak engine iteration 'close-up shop' runtime path https://review.openstack.org/116544 | 01:16 |
openstackgerrit | Joshua Harlow proposed a change to openstack/taskflow: Translate the engine runner into a well defined state-machine https://review.openstack.org/106535 | 01:16 |
openstackgerrit | Joshua Harlow proposed a change to openstack/taskflow: Move parts of action engine tests to a subdirectory https://review.openstack.org/117406 | 01:16 |
openstackgerrit | Joshua Harlow proposed a change to openstack/taskflow: Remove the dependency on prettytable https://review.openstack.org/117629 | 01:18 |
*** dims has quit IRC | 01:19 | |
openstackgerrit | Joshua Harlow proposed a change to openstack/taskflow: Increase/adjust the logging of the WBE response/send activities https://review.openstack.org/102959 | 01:19 |
openstackgerrit | Joshua Harlow proposed a change to openstack/taskflow: Tidy up the WBE cache (now WBE types) module https://review.openstack.org/103245 | 01:19 |
*** takedakn has quit IRC | 01:19 | |
openstackgerrit | Joshua Harlow proposed a change to openstack/taskflow: Ensure state machine can be frozen https://review.openstack.org/107479 | 01:19 |
openstackgerrit | Joshua Harlow proposed a change to openstack/taskflow: Add existing types to generated documentation https://review.openstack.org/106930 | 01:20 |
*** dims has joined #openstack-oslo | 01:20 | |
*** arnaud has joined #openstack-oslo | 01:20 | |
openstackgerrit | Joshua Harlow proposed a change to openstack/taskflow: Update transitioning function name to be more understandable https://review.openstack.org/117392 | 01:20 |
openstackgerrit | Joshua Harlow proposed a change to openstack/taskflow: Explicitly require simplejson https://review.openstack.org/118066 | 01:21 |
openstackgerrit | Joshua Harlow proposed a change to openstack/taskflow: Updated from global requirements https://review.openstack.org/117424 | 01:23 |
openstackgerrit | Davanum Srinivas (dims) proposed a change to openstack/oslo.vmware: Add docs target and generate api docs https://review.openstack.org/118905 | 01:30 |
openstackgerrit | Davanum Srinivas (dims) proposed a change to openstack/oslo.vmware: Add docs target and generate api docs https://review.openstack.org/118905 | 01:32 |
*** arnaud__ has joined #openstack-oslo | 01:32 | |
openstackgerrit | Davanum Srinivas (dims) proposed a change to openstack/oslo.utils: Auto generate the API documentation https://review.openstack.org/119097 | 01:34 |
openstackgerrit | Davanum Srinivas (dims) proposed a change to openstack/oslo.middleware: Auto generate the API documentation https://review.openstack.org/119098 | 01:37 |
openstackgerrit | Davanum Srinivas (dims) proposed a change to openstack/oslo.messaging: Auto generate the API documentation https://review.openstack.org/119099 | 01:44 |
*** dims has quit IRC | 01:46 | |
*** dims has joined #openstack-oslo | 01:47 | |
*** yamahata has joined #openstack-oslo | 01:47 | |
*** dims_ has joined #openstack-oslo | 01:49 | |
*** dims has quit IRC | 01:52 | |
*** praneshp has quit IRC | 02:01 | |
openstackgerrit | Davanum Srinivas (dims) proposed a change to openstack/oslo-incubator: Switch to using pbr's autodoc capability https://review.openstack.org/119130 | 02:04 |
*** takedakn has joined #openstack-oslo | 02:05 | |
*** praneshp has joined #openstack-oslo | 02:06 | |
openstackgerrit | A change was merged to openstack/oslo.vmware: Add exception for TaskInProgress https://review.openstack.org/117254 | 02:07 |
*** _amrith_ is now known as amrith | 02:22 | |
*** dims_ has quit IRC | 02:25 | |
*** dims has joined #openstack-oslo | 02:25 | |
*** harlowja is now known as harlowja_away | 02:27 | |
*** dims_ has joined #openstack-oslo | 02:30 | |
*** dims has quit IRC | 02:30 | |
openstackgerrit | A change was merged to openstack/oslo-incubator: Use MultiStrOpt for policy_dirs https://review.openstack.org/119160 | 02:33 |
openstackgerrit | Davanum Srinivas (dims) proposed a change to openstack/oslo.concurrency: Add code/api documentation https://review.openstack.org/118933 | 02:49 |
openstackgerrit | Davanum Srinivas (dims) proposed a change to openstack/oslo.db: Add code/api documentation https://review.openstack.org/118895 | 02:53 |
*** takedakn has quit IRC | 02:55 | |
*** arnaud__ has quit IRC | 02:55 | |
*** arnaud has quit IRC | 02:55 | |
*** dims_ has quit IRC | 03:04 | |
*** dims has joined #openstack-oslo | 03:05 | |
*** dims has quit IRC | 03:10 | |
*** praneshp has quit IRC | 03:32 | |
*** jecarey has joined #openstack-oslo | 03:42 | |
*** jecarey has quit IRC | 04:08 | |
*** jecarey has joined #openstack-oslo | 04:10 | |
*** arnaud has joined #openstack-oslo | 04:16 | |
*** harlowja_at_home has joined #openstack-oslo | 04:18 | |
*** praneshp has joined #openstack-oslo | 04:20 | |
openstackgerrit | Joshua Harlow proposed a change to openstack/taskflow: Tidy up the WBE cache (now WBE types) module https://review.openstack.org/103245 | 04:24 |
*** praneshp_ has joined #openstack-oslo | 04:24 | |
*** jecarey has quit IRC | 04:25 | |
*** praneshp has quit IRC | 04:27 | |
*** praneshp_ is now known as praneshp | 04:27 | |
*** kevinbenton has quit IRC | 05:03 | |
*** kevinbenton has joined #openstack-oslo | 05:06 | |
*** zarric has joined #openstack-oslo | 05:10 | |
openstackgerrit | Joshua Harlow proposed a change to openstack/taskflow: Ensure that details always exist in the memory backend https://review.openstack.org/119291 | 05:24 |
*** takedakn has joined #openstack-oslo | 05:29 | |
*** flaper87|afk is now known as flaper87 | 05:29 | |
openstackgerrit | Joshua Harlow proposed a change to openstack/taskflow: warn against sorting requirements https://review.openstack.org/118731 | 05:33 |
openstackgerrit | Joshua Harlow proposed a change to openstack/taskflow: warn against sorting requirements https://review.openstack.org/118731 | 05:34 |
*** takedakn has quit IRC | 05:46 | |
openstackgerrit | Michael Davies proposed a change to openstack/cliff: mock.assert_called_once() is not a valid method https://review.openstack.org/119252 | 05:49 |
*** amotoki has joined #openstack-oslo | 05:52 | |
openstackgerrit | OpenStack Proposal Bot proposed a change to openstack/oslo.i18n: Imported Translations from Transifex https://review.openstack.org/116553 | 06:00 |
openstackgerrit | OpenStack Proposal Bot proposed a change to openstack/oslo.db: Imported Translations from Transifex https://review.openstack.org/118539 | 06:01 |
*** stevemar has quit IRC | 06:03 | |
*** takedakn has joined #openstack-oslo | 06:03 | |
openstackgerrit | OpenStack Proposal Bot proposed a change to openstack/oslo.log: Imported Translations from Transifex https://review.openstack.org/118979 | 06:03 |
*** harlowja_at_home has quit IRC | 06:04 | |
*** oomichi has quit IRC | 06:05 | |
openstackgerrit | OpenStack Proposal Bot proposed a change to openstack/oslo.vmware: Imported Translations from Transifex https://review.openstack.org/119296 | 06:06 |
openstackgerrit | OpenStack Proposal Bot proposed a change to openstack/oslo.concurrency: Imported Translations from Transifex https://review.openstack.org/118545 | 06:06 |
openstackgerrit | OpenStack Proposal Bot proposed a change to openstack/oslo.messaging: Imported Translations from Transifex https://review.openstack.org/118547 | 06:08 |
openstackgerrit | OpenStack Proposal Bot proposed a change to openstack/oslo.utils: Imported Translations from Transifex https://review.openstack.org/118549 | 06:09 |
*** takedakn has quit IRC | 06:17 | |
*** k4n0 has joined #openstack-oslo | 06:25 | |
*** jaosorior has joined #openstack-oslo | 06:25 | |
*** takedakn has joined #openstack-oslo | 06:34 | |
*** arnaud has quit IRC | 06:47 | |
*** AAzza has joined #openstack-oslo | 06:49 | |
openstackgerrit | A change was merged to openstack/oslo.concurrency: warn against sorting requirements https://review.openstack.org/118716 | 07:13 |
*** pblaho has joined #openstack-oslo | 07:29 | |
*** takedakn has quit IRC | 07:32 | |
*** ajo|call is now known as ajo | 07:44 | |
*** stannie1 has joined #openstack-oslo | 07:45 | |
*** YorikSar has quit IRC | 07:48 | |
*** YorikSar has joined #openstack-oslo | 07:50 | |
*** YorikSar has quit IRC | 07:56 | |
*** sreshetnyak has quit IRC | 07:58 | |
*** sreshetnyak has joined #openstack-oslo | 08:05 | |
*** YorikSar has joined #openstack-oslo | 08:06 | |
*** YorikSar_ has joined #openstack-oslo | 08:12 | |
*** dtantsur|afk is now known as dtantsur | 08:15 | |
*** YorikSar has quit IRC | 08:16 | |
openstackgerrit | jazeltq proposed a change to openstack/oslo-incubator: fix memory leak for _safe_log https://review.openstack.org/119317 | 08:23 |
*** pblaho is now known as pblaho|afk | 08:24 | |
openstackgerrit | ChangBo Guo(gcb) proposed a change to openstack/oslo-incubator: Add module integrity check in update.py https://review.openstack.org/76901 | 08:27 |
*** praneshp has quit IRC | 08:33 | |
*** pblaho|afk is now known as pblaho | 08:38 | |
*** i159 has joined #openstack-oslo | 08:41 | |
openstackgerrit | Ann Kamyshnikova proposed a change to openstack/oslo.db: ModelsMigrationsSync: Add check for foreign keys https://review.openstack.org/116238 | 08:49 |
*** alexpilotti has joined #openstack-oslo | 08:50 | |
*** dtantsur is now known as dtantsur|brb | 09:10 | |
*** k4n0 has quit IRC | 09:16 | |
*** ildikov has joined #openstack-oslo | 09:20 | |
*** YorikSar_ has quit IRC | 09:31 | |
*** YorikSar has joined #openstack-oslo | 09:32 | |
openstackgerrit | Oleksii Chuprykov proposed a change to openstack/oslo.db: Renaming in WalkVersionsMixin https://review.openstack.org/119078 | 09:34 |
*** GheRivero is now known as GheRivero_pto | 09:34 | |
*** pblaho is now known as pblaho|afk | 10:01 | |
*** pcm_ has joined #openstack-oslo | 10:14 | |
*** pcm_ has quit IRC | 10:16 | |
*** pcm_ has joined #openstack-oslo | 10:16 | |
*** AAzza is now known as AAzza_afk | 10:18 | |
openstackgerrit | A change was merged to openstack-dev/pbr: Accept capitalized Sem-Ver headers https://review.openstack.org/118648 | 10:18 |
openstackgerrit | A change was merged to openstack/oslo.middleware: warn against sorting requirements https://review.openstack.org/118725 | 10:18 |
openstackgerrit | A change was merged to openstack/oslo.middleware: Work toward Python 3.4 support and testing https://review.openstack.org/118791 | 10:18 |
openstackgerrit | A change was merged to openstack/oslo.vmware: warn against sorting requirements https://review.openstack.org/118729 | 10:19 |
*** pblaho|afk is now known as pblaho | 10:31 | |
*** AAzza_afk is now known as AAzza | 10:37 | |
*** yamahata has quit IRC | 10:46 | |
*** dims has joined #openstack-oslo | 10:54 | |
*** dims has quit IRC | 10:56 | |
*** dims_ has joined #openstack-oslo | 10:57 | |
*** dims has joined #openstack-oslo | 10:57 | |
*** ildikov_ has joined #openstack-oslo | 11:00 | |
*** dims_ has quit IRC | 11:01 | |
*** dtantsur|brb is now known as dtantsur | 11:01 | |
*** ildikov has quit IRC | 11:03 | |
*** tongli has joined #openstack-oslo | 11:30 | |
*** amrith is now known as _amrith_ | 11:45 | |
*** k4n0 has joined #openstack-oslo | 11:55 | |
*** AAzza is now known as AAzza_afk | 11:55 | |
*** openstackgerrit has quit IRC | 12:01 | |
*** openstackgerrit has joined #openstack-oslo | 12:02 | |
*** dims has quit IRC | 12:19 | |
*** dims has joined #openstack-oslo | 12:20 | |
openstackgerrit | Davanum Srinivas (dims) proposed a change to openstack/oslo-incubator: Add contributing page to docs https://review.openstack.org/119359 | 12:25 |
openstackgerrit | Davanum Srinivas (dims) proposed a change to openstack/oslo.config: Add contributing page to docs https://review.openstack.org/119360 | 12:25 |
openstackgerrit | Davanum Srinivas (dims) proposed a change to openstack/oslo.messaging: Add contributing page to docs https://review.openstack.org/119361 | 12:26 |
*** YorikSar has quit IRC | 12:26 | |
*** ttx has quit IRC | 12:31 | |
*** jaypipes is now known as leakypipes | 12:32 | |
*** YorikSar has joined #openstack-oslo | 12:33 | |
openstackgerrit | Oleksii Chuprykov proposed a change to openstack/oslo.db: Renaming in WalkVersionsMixin https://review.openstack.org/119078 | 12:33 |
*** ttx has joined #openstack-oslo | 12:34 | |
*** ttx has quit IRC | 12:34 | |
*** ttx has joined #openstack-oslo | 12:34 | |
*** dims has quit IRC | 12:39 | |
*** dims has joined #openstack-oslo | 12:40 | |
*** lbragstad has quit IRC | 12:43 | |
*** lbragstad has joined #openstack-oslo | 12:43 | |
*** dims has quit IRC | 12:45 | |
*** viktors|afk is now known as viktors | 12:45 | |
*** amotoki has quit IRC | 12:45 | |
*** dims_ has joined #openstack-oslo | 12:46 | |
openstackgerrit | Oleksii Chuprykov proposed a change to openstack/oslo.db: Add description for test_models_sync function https://review.openstack.org/116004 | 12:48 |
*** dims_ has quit IRC | 12:49 | |
*** miqui has joined #openstack-oslo | 12:50 | |
*** dims has joined #openstack-oslo | 12:50 | |
openstackgerrit | Doug Hellmann proposed a change to openstack/oslotest: Add fixture for mock.patch.multiple https://review.openstack.org/119085 | 12:51 |
*** morganfainberg has quit IRC | 12:54 | |
*** morganfainberg has joined #openstack-oslo | 12:57 | |
*** _amrith_ is now known as amrith | 13:05 | |
dhellmann | cores: we need to prioritize reviews for oslotest so we can cut a new release and fix the python 3 issues we're having syncing requirements updates: https://review.openstack.org/#/q/project:openstack%2Foslotest+is:open,n,z | 13:14 |
openstackgerrit | A change was merged to openstack/oslotest: Remove differences between Python 2.x and 3.x versions https://review.openstack.org/118551 | 13:21 |
dhellmann | dims, bnemec, flaper87: review requests for oslotest ^^ | 13:22 |
* flaper87 clicks | 13:22 | |
flaper87 | dhellmann: I +2'd the proton driver patch. It's got 2 +2 but I prefered to leave the +A to you. | 13:23 |
flaper87 | I don't recall you explicitly approving the FEE, besides our chats. | 13:23 |
dhellmann | flaper87: yeah, I'll go ahead and do that | 13:23 |
*** mriedem has joined #openstack-oslo | 13:24 | |
*** morganfainberg has quit IRC | 13:25 | |
*** morganfainberg has joined #openstack-oslo | 13:27 | |
flaper87 | dhellmann: https://review.openstack.org/#/c/118734/ needs rebase :/ | 13:29 |
*** bknudson has quit IRC | 13:30 | |
dhellmann | flaper87: ok, that can wait, I'm more worried about the stopall fix and the mock.patch.multiple addition | 13:31 |
flaper87 | dhellmann: yeah, I'm done with those :P | 13:31 |
openstackgerrit | Doug Hellmann proposed a change to openstack/oslotest: warn against sorting requirements https://review.openstack.org/118734 | 13:32 |
dhellmann | flaper87: thanks :-) | 13:32 |
*** dhellmann is now known as dhellmann_ | 13:40 | |
*** pblaho has quit IRC | 13:43 | |
dtantsur | Folks, could someone have a look at cliutils patch https://review.openstack.org/#/c/113978/ ? Already has 2 +1 and is blocking a couple of patches in python-ironicclient. Thanks | 13:49 |
dtantsur | dhellmann_, maybe you could ^^^ | 13:49 |
*** bknudson has joined #openstack-oslo | 13:51 | |
*** jeckersb_gone is now known as jeckersb | 13:51 | |
*** jaosorior has quit IRC | 14:02 | |
*** ildikov has joined #openstack-oslo | 14:02 | |
*** ildikov_ has quit IRC | 14:02 | |
*** bnemec is now known as beekneemech | 14:02 | |
*** i159 has quit IRC | 14:02 | |
*** zzzeek has joined #openstack-oslo | 14:05 | |
*** dims is now known as dimsum_ | 14:12 | |
*** kgiusti has joined #openstack-oslo | 14:13 | |
*** zarric has quit IRC | 14:16 | |
*** zarric has joined #openstack-oslo | 14:18 | |
*** morganfainberg has quit IRC | 14:18 | |
*** morganfainberg has joined #openstack-oslo | 14:27 | |
miqui | which is correct oslo.sphinx or oslophinx? | 14:36 |
openstackgerrit | A change was merged to openstack/oslo-incubator: Add field_labels parameter to cliutils.print_list https://review.openstack.org/113978 | 14:38 |
*** jecarey has joined #openstack-oslo | 14:40 | |
dimsum_ | oslosphinx | 14:42 |
*** mtanino has joined #openstack-oslo | 14:42 | |
amrith | yoriksar, beekneemech ... do you have second? have a question that I'm sure one of you could help me with .. thx | 14:43 |
*** dhellmann_ is now known as dhellmann | 14:43 | |
YorikSar | amrith: Hi. Sure, ask away :) | 14:44 |
amrith | yoriksar, hi | 14:44 |
amrith | here's the question | 14:44 |
amrith | if you look at this test | 14:44 |
amrith | in trove | 14:44 |
amrith | https://github.com/openstack/trove/blob/master/trove/tests/unittests/guestagent/test_pkg.py#L433 | 14:44 |
amrith | the test cases are bad for a couple of reasons | 14:44 |
amrith | the one I'm looking into is the [mis]use of Mock | 14:44 |
amrith | you'll see that these tests Mock pexpect.spawn.expect and pexpect.spawn.match and don't unmock | 14:45 |
amrith | therefore the test performance is erratic | 14:45 |
amrith | if someone mocks .match ahead of the run of test_permission_error, all is well | 14:45 |
amrith | if not, there's a problem | 14:45 |
amrith | I want to use @mock.patch | 14:45 |
amrith | decorators | 14:45 |
amrith | I'm able to get @patch('pexpect.spawn.expect', Mock(return_value=0)) to work | 14:45 |
amrith | but this one doesn't seem to do what I need | 14:45 |
amrith | @patch('pexpect.spawn.match', False) | 14:46 |
openstackgerrit | A change was merged to openstack/oslotest: Cleaning up index.rst file https://review.openstack.org/96100 | 14:46 |
YorikSar | amrith: You shouldn't explicitely create mocks. Almost ever. :) | 14:46 |
amrith | the code being executed is at https://github.com/openstack/trove/blob/master/trove/guestagent/pkg.py | 14:46 |
amrith | I agree | 14:46 |
amrith | this was one of your comments to me in a review (some oslo code) | 14:46 |
amrith | and I remembered that | 14:46 |
amrith | and hence wanted to convert this test case to use decorators | 14:47 |
amrith | and I can't seem to get that to work | 14:47 |
YorikSar | amrith: Most arguments to those mocks can be provided as args to @patch or set up on mock object that is passed to the method in arguments | 14:47 |
beekneemech | amrith: I strongly suggest switching to oslotest as the base class for that test. | 14:47 |
beekneemech | The oslotest base class cleans up mocks automatically. | 14:47 |
dhellmann | amrith: agree with beekneemech, and have a look at the mockpatch fixtures | 14:47 |
beekneemech | Not that it wouldn't be better to just use the decorator in that case. I don't see any reason it wouldn't work. | 14:47 |
*** zz_jgrimm is now known as jgrimm | 14:47 | |
amrith | beekneemech, I'll look at oslotest | 14:48 |
amrith | but, I've torn enough hear off my head in the last day | 14:48 |
amrith | that I want to get to the bottom of this ;) | 14:48 |
YorikSar | amrith: Maybe you meant @patch(..., return_value=False) ? | 14:48 |
amrith | well, match isn't a method | 14:48 |
amrith | OH? | 14:48 |
amrith | let me try that. | 14:48 |
amrith | if it is, I'm going to start drinking early (it's friday ;)) | 14:49 |
openstackgerrit | Davanum Srinivas (dims) proposed a change to openstack/oslo.messaging: Add square brackets for ipv6 based hosts https://review.openstack.org/119389 | 14:49 |
amrith | nope, doesn't work | 14:49 |
amrith | with @patch('pexpect.spawn.match', return_value=False) | 14:49 |
YorikSar | You'll need to expect a new argument to your test. | 14:49 |
openstackgerrit | Davanum Srinivas (dims) proposed a change to openstack/oslo.messaging: Add square brackets for ipv6 based hosts https://review.openstack.org/119389 | 14:49 |
beekneemech | If match isn't a function, I don't think patching it will work though. return_value implies it will be called, not used directly. | 14:50 |
openstackgerrit | Davanum Srinivas (dims) proposed a change to openstack/oslo.messaging: Add square brackets for ipv6 based hosts https://review.openstack.org/119389 | 14:50 |
amrith | YorikSar, would you be able to point me to an example. | 14:50 |
amrith | beekneemech, yes, I think that's what's happening | 14:50 |
amrith | and why the other tests just hammer pexpect.spawn.match = False | 14:50 |
amrith | and go on their merry way | 14:50 |
YorikSar | amrith: Em... Ok, I don't know what pexpect is. If it's not a method, you should be able to just pass a new value there. | 14:50 |
amrith | that seems WRONG! | 14:50 |
amrith | pexpect.spawn.expect() is a method | 14:51 |
beekneemech | amrith: You should probably save the original value of match in setUp and restore it in tearDown. | 14:51 |
amrith | pexpect.spawn.match is a value | 14:51 |
beekneemech | or as a cleanup | 14:51 |
amrith | beekneemech, that would be easy to do | 14:51 |
YorikSar | beekneemech: mock should do just fine with non-methods... | 14:51 |
amrith | except that in some cases, (https://github.com/openstack/trove/blob/master/trove/tests/unittests/guestagent/test_pkg.py#L339) the value is not True/False but a call to re.match(...) | 14:51 |
amrith | I was going to make that a side_effect | 14:52 |
amrith | and be done with it | 14:52 |
amrith | the thing that is bugging me is why pexpect.spawn.match being set to False doesn't work ;) | 14:52 |
YorikSar | amrith: No, you can't use side_effect htere. | 14:52 |
amrith | what isn't helping me (also) is the fact that I get only three lines of backtrace https://gist.github.com/amrith/96356c0e8b964651ab00 | 14:53 |
amrith | I'd love to know why that is, and who's chopping the backtrace to the most useless part. | 14:53 |
YorikSar | amrith: You should use patch as context manager if you can't determine new value at class declaration time. | 14:53 |
amrith | YorikSar, ok. that makes sense. I think. but isn't that equivalent to having it be defined as a side effect. | 14:54 |
amrith | i.e. they accomplish the same thing | 14:54 |
amrith | in different ways? | 14:54 |
YorikSar | amrith: Eh... Looks like mock doesn't like the fact that there's no 'match' attribute at all. | 14:54 |
amrith | yes | 14:54 |
amrith | but that's what I'm setting in the @patch ;) | 14:54 |
YorikSar | amrith: No, side_effect happens when you call a mock. match isn't called, it's just read. | 14:54 |
amrith | well, clearly someone is trying to read and can't seem to find it | 14:55 |
amrith | what I don't know is this | 14:55 |
amrith | is the thing trying to read/write match the code doing the @patch() | 14:55 |
amrith | or the code in pexpect_run that is trying to read 'match' | 14:55 |
*** flaper87 is now known as flaper87|afk | 14:55 | |
amrith | and the truncation of the backtrace isn't helpful | 14:55 |
YorikSar | amrith: Well, then you have to do mocking by yourself in setUp and properly handle the case when there's no 'match' attribute there. | 14:56 |
beekneemech | amrith: It looks like match is set after you call expect, so I'm not sure you can mock it ahead of time: http://pexpect.sourceforge.net/pexpect.html#spawn | 14:56 |
beekneemech | I'm reading the expect docstring and it says "After a match is found the instance attributes 'before', 'after' and | 14:56 |
beekneemech | 'match' will be set." | 14:56 |
beekneemech | So your mock is probably being overwritten anyway. | 14:56 |
amrith | beekneemech, if what you say is true then the error I'd get should be that match is found but true or something | 14:57 |
amrith | but also, the current way of doing it, which is to hammer it to False ahead of time shouldn't work either, no? | 14:57 |
*** k4n0 has quit IRC | 14:57 | |
YorikSar | amrith: Ok, here's another idea. Is pexpect.spawn used for anything but calling expect and match? | 14:57 |
amrith | not that I can see | 14:57 |
amrith | will you suggest mocking that? | 14:57 |
*** stevemar has joined #openstack-oslo | 14:57 | |
YorikSar | amrith: If not, you should patch pexpect.spawn and set its attributes. | 14:57 |
amrith | so, I updated the gist above to include the code as I have it now. | 14:58 |
amrith | I see ... | 14:58 |
amrith | patch pexpect.spawn | 14:58 |
amrith | and then set it's attributes | 14:58 |
amrith | can that be done as a decorator? | 14:59 |
YorikSar | amrith: Oh, it's even patched all over in tests: __init__ and close are patched in setUp | 14:59 |
amrith | yes, this is a regular dogs breakfast ... | 14:59 |
YorikSar | I think you should use MockPatch fixture in setUp instead and let tests set pexpect.spawn.match as they do now. | 15:00 |
beekneemech | brb | 15:01 |
amrith | YorikSar, I get the use MockPatch part. but having each test set the match directly seems bad. | 15:01 |
amrith | let me try one thing. | 15:01 |
*** beekneemech has quit IRC | 15:01 | |
*** bnemec has joined #openstack-oslo | 15:02 | |
amrith | so, the theory that I can't set pexpect.spawn.match early is wrong because if I hammer it to False in setUp, it works fine | 15:02 |
YorikSar | amrith: They'll be setting match attribute of a mock object, it's fine. | 15:02 |
amrith | well, in that case, how can I do that in a decorator? | 15:02 |
*** bnemec is now known as beekneemech | 15:03 | |
amrith | I guess the question boils down to this | 15:03 |
amrith | I am able to mock a method | 15:03 |
amrith | but not an attribute | 15:03 |
*** palendae has joined #openstack-oslo | 15:03 | |
beekneemech | amrith: The mock decorator passes in the mock object. You just set the attribute on that object. | 15:03 |
*** r1chardj0n3s has joined #openstack-oslo | 15:04 | |
amrith | @patch('pexpect.spawn.match', False) | 15:04 |
amrith | does not appear to work | 15:04 |
r1chardj0n3s | hai dhellmann :) | 15:05 |
YorikSar | amrith: You'll have to pass mock object to an argument and do the same - set match attribute on it. | 15:05 |
amrith | YorikSar, OK. Understood. I'll try that | 15:05 |
amrith | and then switch to the fixture as you both suggested | 15:05 |
amrith | Thanks muchly! | 15:05 |
*** yamahata has joined #openstack-oslo | 15:07 | |
YorikSar | amrith: Here's a scetch of what I'm proposing (w/o fixture): http://paste.openstack.org/show/106586/ | 15:08 |
* amrith goes to look | 15:08 | |
YorikSar | amrith: Although using fixture would be better, but you'll need oslotest for that. | 15:08 |
amrith | YorikSar, that looks perfect. I'm going to try it for one test (class) and see how it works out. | 15:10 |
openstackgerrit | A change was merged to openstack/oslotest: Ensure that mock.patch.stopall is called last https://review.openstack.org/119201 | 15:10 |
amrith | YorikSar, would you leave the existing hacking of __init__ and .closed in setUp() alone? | 15:13 |
YorikSar | amrith: You don't need it if you replace the whole thing with mock. | 15:13 |
YorikSar | amrith: __init__ and closed will already be mocked. | 15:14 |
amrith | got it | 15:14 |
*** i159 has joined #openstack-oslo | 15:19 | |
dhellmann | r1chardj0n3s: hi! | 15:19 |
r1chardj0n3s | hey! so I would like to resurrect https://review.openstack.org/#/c/21958/ :) | 15:19 |
dhellmann | wow, that's an old one | 15:20 |
r1chardj0n3s | yeah, and I wish it had been finished so I did't have to do it, but them's the breaks ;) | 15:20 |
dhellmann | that should probably be submitted to oslo.middleware now, instead of the incubator | 15:20 |
r1chardj0n3s | ok, I can look at making a new change against middleware | 15:20 |
dhellmann | are you looking for a FFE for juno, or is this early kilo work? | 15:20 |
r1chardj0n3s | kilo | 15:21 |
dhellmann | cool | 15:21 |
r1chardj0n3s | I have A Plan | 15:21 |
r1chardj0n3s | :) | 15:21 |
dhellmann | I'll retarget the blueprint, should I go ahead and assign it to you? | 15:21 |
dhellmann | hmm, I don't actually see a blueprint called cors-support | 15:21 |
r1chardj0n3s | me either | 15:22 |
r1chardj0n3s | I can write a new one if necessary | 15:22 |
dhellmann | yeah, it's probably not controversial, but it will help the reviewers if there is some background on what this is for | 15:22 |
r1chardj0n3s | yep, ok | 15:23 |
dhellmann | https://bugs.launchpad.net/oslo.middleware/+bug/987044 is real | 15:23 |
r1chardj0n3s | ahah! | 15:23 |
dhellmann | I'm going to remove the reference to the incubator, since the work will just happen in the lib | 15:23 |
r1chardj0n3s | ok, I can claim that. so you think a blueprint might be optional for this? | 15:23 |
dhellmann | r1chardj0n3s: yeah, this seems small enough that a blueprint is optional, but we'll want to make sure we target the bug so it gets reviews | 15:25 |
r1chardj0n3s | dhellmann: ok | 15:26 |
amrith | YorikSar, doesn't appear to work (with your proposed code). I'll wait for r1chardj0n3s and dhellmann to finish chatting ... | 15:26 |
r1chardj0n3s | dhellmann: just going through the death-throes of a similar proposal over in keystone land before I start on it in olso | 15:27 |
dhellmann | r1chardj0n3s: if this is controversial, it might be worth bringing it up on the -dev list to resolve questions/issues | 15:27 |
dhellmann | r1chardj0n3s: I've targeted the bug for next-kilo, what's your launchpad id? | 15:28 |
r1chardj0n3s | dhellmann: "richard" | 15:28 |
dhellmann | that is associated with the name "Richard Samson", is that you? | 15:29 |
r1chardj0n3s | er | 15:29 |
r1chardj0n3s | ah, sorry, r1chardjones | 15:29 |
r1chardj0n3s | ugh cannot type own thing | 15:29 |
r1chardj0n3s | same as IRC nick | 15:29 |
r1chardj0n3s | sorry :) | 15:29 |
dhellmann | ok, I think I assigned it to you (should have guessed) | 15:30 |
r1chardj0n3s | I got "clever" and used "richard" on gerrit :/ | 15:30 |
*** alexpilotti has quit IRC | 15:30 | |
dhellmann | ah | 15:30 |
dhellmann | adding a piece of middleware is going to be the easy part of this, but if you want the other projects to actually use it you might need more buy-in | 15:30 |
r1chardj0n3s | "horizon is terrible"? :) | 15:31 |
r1chardj0n3s | it's a relatively small impact thing, and I think I can swing enough votes to make it a reality | 15:32 |
dhellmann | ok, I just don't want you to underestimate the amount of effort making a change in every project might take | 15:32 |
r1chardj0n3s | heh, I still have to integrate pip-missing-reqs in everything ;) | 15:33 |
amrith | YorikSar, the model your proposed (http://paste.openstack.org/show/106586/) does not work it appears that even though I set pexpect.spawn.expect.return_value = 0 in the test, when we get into the code being tested, pexpect.spawn() returns child and child.expect() returns not 0 but <MagicMock name='spawn().expect()' id='139866981895888'> | 15:33 |
amrith | I'll try something else and get back to you | 15:33 |
amrith | thanks for your help | 15:33 |
YorikSar | amrith: Oh, then you should do pexpect.spawn.return_value.expect.return_value = 0 | 15:34 |
dhellmann | beekneemech: do you have a few minutes to look at an oslotest enhancement for oslo.serialization? https://review.openstack.org/#/c/119085/ | 15:34 |
YorikSar | amrith: I missed the fact that spawn is being called. | 15:34 |
amrith | YorikSar, ... let me try that. | 15:34 |
dhellmann | r1chardj0n3s: that looks like it could be useful :-) | 15:35 |
r1chardj0n3s | dhellmann: it's now in nova and found one missing requirement there ;) | 15:35 |
dhellmann | r1chardj0n3s: I look forward to your patches in the oslo repositories | 15:35 |
*** r1chardj0n3s is now known as r1chardj0n3s_afk | 15:36 | |
r1chardj0n3s_afk | dhellmann: thanks :) | 15:36 |
*** dtantsur is now known as dtantsur|afk | 15:36 | |
dhellmann | r1chardj0n3s: although interestingly, we had been planning to ask projects to *not* list depdendencies brought in if they depend on oslo libs, to cut down on the number of places that have to be updated when we bump a requirement | 15:36 |
amrith | YorikSar, that worked. Thanks much. | 15:36 |
amrith | I'll go figure the fixture out now | 15:36 |
amrith | YorikSar, beekneemech thanks so much! | 15:36 |
beekneemech | dhellmann: Yeah, I've been trying to review that this morning. :-) | 15:37 |
dhellmann | beekneemech: thanks, that one is holding up fixing oslotest's packaging so we can update requirements in a bunch of the other projects, including oslo.serialization | 15:38 |
*** AAzza_afk is now known as AAzza | 15:41 | |
*** markmcclain has joined #openstack-oslo | 15:41 | |
*** yamahata has quit IRC | 15:42 | |
*** yamahata has joined #openstack-oslo | 15:42 | |
*** r1chardj0n3s_afk is now known as r1chardj0n3s | 15:43 | |
*** flaper87|afk is now known as flaper87 | 15:43 | |
r1chardj0n3s | dhellmann: pip-missing-reqs is about packages that are explicitly used in a codebase but not explicitly listed in requirements.txt - so if the implicit package chain in requirements is broken the code doesn't break :) | 15:44 |
*** markmcclain1 has joined #openstack-oslo | 15:45 | |
dhellmann | r1chardj0n3s: ok, that makes sense | 15:45 |
*** markmcclain has quit IRC | 15:46 | |
dhellmann | team meeting in ~15 minutes in #openstack-meeting-alt | 15:46 |
r1chardj0n3s | dhellmann: please correct me if I'm wrong, but I don't think adding CORS middleware to oslo and then using that in individual APIs will require any change to those APIs - it's purely config-file (install-time) change for them | 15:50 |
r1chardj0n3s | oh, except there's the requirements.txt change... | 15:50 |
beekneemech | dhellmann: Okay, https://review.openstack.org/#/c/119085/ +2a | 15:51 |
* beekneemech is refreshing his mock knowledge this morning :-) | 15:51 | |
dhellmann | r1chardj0n3s: possibly requirements change, certainly documentation suggesting its use, and a lot of the projects still ship default paste config files | 15:52 |
r1chardj0n3s | yep, ok | 15:52 |
dhellmann | r1chardj0n3s: you'd want it enabled in devstack, too, for testing | 15:52 |
r1chardj0n3s | absolutely, yep | 15:52 |
dhellmann | r1chardj0n3s: and that's all without me having really looked at the code to see what it's doing (if it's just adding headers or something that's one thing, but some projects aren't using webob so if you have to do anything with the incoming request that may be a challenge) | 15:53 |
dhellmann | r1chardj0n3s: oh, and ceilometer doesn't use paste so if you want it in by default you probably want to just add it to their app setup module. Ironic (and other pecan users) might be the same | 15:53 |
dhellmann | beekneemech: thanks | 15:54 |
r1chardj0n3s | dhellmann: yikes :) | 15:54 |
dhellmann | r1chardj0n3s: we were trying to get the apps to recognize paste as a deployer tool and not an app developer tool, though you may disagree with that assessment | 15:54 |
dhellmann | i.e., separate the "this middleware is actually part of the application" definition from "this deployer has wrapped an app with something" | 15:55 |
r1chardj0n3s | dhellmann: opting into CORS is a deployer thing, I believe, but having the tool in place to make it easy for deployers to opt in is an app thing :) | 15:56 |
dhellmann | nothing prevents deployers from choosing to use paste, so I think we're good | 15:56 |
r1chardj0n3s | and here I thought the difficult part would be re-writing Horizon in angularjs | 15:56 |
r1chardj0n3s | :) | 15:57 |
r1chardj0n3s | I think a spec might be useful if only to point people at if per-project changes are necessary | 15:57 |
r1chardj0n3s | so I will do that first | 15:57 |
r1chardj0n3s | actually, I will coffee first | 15:58 |
dhellmann | that seems wise | 15:58 |
dhellmann | also that :-) | 15:58 |
dimsum_ | dhellmann: quick question, who/how is ChangeLog file generated? | 15:59 |
dhellmann | dimsum_: pbr generates that when installing or building the sdist file | 15:59 |
dhellmann | dimsum_: it works for our doc builds because they happen in tox, so our libs are installed before sphinx runs | 15:59 |
dimsum_ | dhellmann: i am running tox -e docs in oslo.rootwrap for example and i don't see it get generated | 16:00 |
dhellmann | ok, I'll look at it with you after the meeting | 16:01 |
openstackgerrit | Victor Sergeyev proposed a change to openstack/oslo.db: Fix DBReferenceError on MySQL and SQLite https://review.openstack.org/116324 | 16:02 |
*** dims_ has joined #openstack-oslo | 16:09 | |
*** dims_ has quit IRC | 16:09 | |
*** dims_ has joined #openstack-oslo | 16:10 | |
*** dims_ has quit IRC | 16:10 | |
*** dims has joined #openstack-oslo | 16:11 | |
*** dims has quit IRC | 16:11 | |
*** amrith is now known as _amrith_ | 16:12 | |
*** alexpilotti has joined #openstack-oslo | 16:12 | |
*** dims_ has joined #openstack-oslo | 16:13 | |
*** dimsum_ has quit IRC | 16:14 | |
*** dims_ is now known as dimsum_ | 16:14 | |
morganfainberg | zzzeek, i think it is related to that thread local stuff (the client re-use) | 16:14 |
morganfainberg | zzzeek in memcached | 16:14 |
zzzeek | yup | 16:14 |
*** dimsum_ has quit IRC | 16:14 | |
morganfainberg | zzzeek, not sure the easiest way to solve it for keystone though. | 16:15 |
morganfainberg | zzzeek, unless you have an idea for it from a larger standpoint. | 16:15 |
*** dimsum_ has joined #openstack-oslo | 16:16 | |
zzzeek | morganfainberg: I dont have a full view of the nature of hte issue. A Python threadlocal should clean out state from threads that have died | 16:17 |
*** dimsum_ has quit IRC | 16:17 | |
morganfainberg | zzzeek, it might be as simple as under load GC delays and socket holding (kernel wait states) are biting them | 16:17 |
zzzeek | that is possible. threadlocals aren’t a great path to “scale” :) | 16:17 |
morganfainberg | exactly | 16:18 |
morganfainberg | zzzeek, we did something like this: https://github.com/openstack/keystonemiddleware/blob/master/keystonemiddleware/auth_token.py#L1085-L1109 for middleware. it's subwonderful for a number of reasons but solves the issue under eventlet-based memcache requesting | 16:19 |
morganfainberg | it should never allocate more than the number of active greenthreads. but... errors in the connections could get ugly | 16:19 |
zzzeek | i think we need a memcache client that doesnt have a threadlocal hardcoded into it | 16:19 |
morganfainberg | zzzeek, ++ | 16:19 |
morganfainberg | probably | 16:19 |
morganfainberg | i also *hate* python's memcache libraries. | 16:20 |
morganfainberg | ok hate is a bit strong | 16:20 |
morganfainberg | dislike with enthusiasm | 16:20 |
morganfainberg | :P | 16:20 |
morganfainberg | there are a lot of weird edge cases i've seen over the years. | 16:20 |
morganfainberg | hm | 16:22 |
morganfainberg | but more importantly, keystone runs single eventlet worker in Icehouse. | 16:22 |
morganfainberg | this might just be rooted in eventlet weirdness. | 16:22 |
*** praneshp has joined #openstack-oslo | 16:22 | |
zzzeek | not really sure :) | 16:23 |
zzzeek | i havent worked on this issue | 16:24 |
*** leakypipes has quit IRC | 16:24 | |
*** dims has joined #openstack-oslo | 16:24 | |
*** dims has quit IRC | 16:24 | |
morganfainberg | i'll try and get a devstack stood up and duplicate the issue. not sure how successful i'll be tbh though | 16:26 |
*** flaper87 is now known as flaper87|afk | 16:28 | |
*** flaper87|afk is now known as flaper87 | 16:31 | |
openstackgerrit | A change was merged to openstack/oslotest: Add fixture for mock.patch.multiple https://review.openstack.org/119085 | 16:32 |
*** dims has joined #openstack-oslo | 16:33 | |
*** yamahata has quit IRC | 16:33 | |
morganfainberg | zzzeek, confirmed eventlet breaks threadlocal magic | 16:48 |
YorikSar | flaper87: What do you think about https://review.openstack.org/112669 (futures-based ReplyWaiter)? (not to disrupt meeting) | 16:48 |
morganfainberg | zzzeek, under apache no issue we sit at one per worker | 16:48 |
morganfainberg | zzzeek, under eventlet we spiral out of control and leak connections like crazy | 16:48 |
morganfainberg | zzzeek, i think the solution is going to be using a pool like keystonemiddleware does | 16:49 |
zzzeek | morganfainberg: ok how does that work around this: https://github.com/linsomniac/python-memcached/blob/master/memcache.py#L129 | 16:49 |
zzzeek | the whole Client is a threading.local | 16:49 |
zzzeek | so everything in it, is a thread local | 16:49 |
morganfainberg | zzzeek, i think it's because we're wrapping the object client. | 16:50 |
morganfainberg | zzzeek, i *think*. | 16:50 |
zzzeek | its a horrible design | 16:50 |
morganfainberg | zzzeek, yeah. | 16:50 |
zzzeek | if you are in a diffrent thread, you will get brand new state. i dont see how you can “wrap” that | 16:50 |
morganfainberg | zzzeek, but basically we're leaking a ton of clients under eventlet. | 16:50 |
morganfainberg | zzzeek, it's eventlet making things look like new contexts each request | 16:50 |
zzzeek | morganfainberg: i think if we cant cahnge this client we shoudl be monkeypatching Client.__mro__ | 16:50 |
zzzeek | err Client.__bases__ | 16:51 |
morganfainberg | zzzeek, ughhhhhhhhhh | 16:51 |
morganfainberg | zzzeek, probably :( | 16:51 |
YorikSar | morganfainberg, zzzeek: I've implemented a backend for dogpile that would craete connection pool and strip threading.local off python-memcached's Client. | 16:51 |
zzzeek | morganfainberg: its a bug in memcached | 16:51 |
zzzeek | YorikSar: you patch Client.__bases__ ? :) | 16:51 |
YorikSar | I might take some time to creae pull requrests to python-memcached and dogpile.cache | 16:51 |
YorikSar | zzzeek: Not exactly... | 16:51 |
zzzeek | YorikSar: work on python-memcached :) | 16:52 |
*** jogo is now known as flashgordon | 16:52 | |
morganfainberg | YorikSar, have some example code? | 16:52 |
morganfainberg | i'm curious. | 16:52 |
morganfainberg | and yes python-memcached needs to be fixed but likely we need to provide a fix at least in keystone until we get upstream fixed (and backported to icehouse) | 16:52 |
YorikSar | MemcacheClient = type('MemcacheClient', (object,), dict(memcache.Client.__dict__)) | 16:53 |
YorikSar | morganfainberg, zzzeek: But that should be a separate class in python-memcached. | 16:53 |
zzzeek | YorikSar: yeah you can also swap MemcacheClient.__bases__. might be better. but yes, python-memcached should not do this by default, or provide an optoin, or something | 16:54 |
zzzeek | morganfainberg: yes, but you need this monkeypatch to fix for now. otherwise its still a thread local | 16:54 |
morganfainberg | zzzeek, *sigh* yeah ickyness. but at least we know what is going on | 16:55 |
YorikSar | zzzeek: Would you accept pull request with my connection pool to dogpile.cache (once python-memcached provides non-threadlocal Client)? | 16:56 |
zzzeek | YorikSar: id have to see it. does the pool work with any of the memcache backends ? | 16:57 |
YorikSar | zzzeek: It's as abstract as it gets. So we can use it with any backend (just as threadlocal one) | 16:57 |
zzzeek | YorikSar: is it somthign that can knid of be an option people can pull in ? taht would be great | 16:57 |
zzzeek | YorikSar: like a wrapper backend | 16:57 |
YorikSar | zzzeek: I think we can add an argument like "which pool to use". | 16:58 |
zzzeek | YorikSar: if its of general use and has good tests and is something people pull in then that’s a good start | 16:58 |
YorikSar | zzzeek: Keep using threadlocal by default and provide some (at least one) options | 16:58 |
zzzeek | YorikSar: hm do i have the threadlocal in dogpile itself ? | 16:58 |
zzzeek | YorikSar: i do dont i | 16:58 |
YorikSar | zzzeek: Yep | 16:59 |
zzzeek | YorikSar: yeah | 16:59 |
zzzeek | YorikSar: :) | 16:59 |
zzzeek | YorikSar: theadlocal on threadlocal, we have to fix that :) | 16:59 |
YorikSar | zzzeek: Exactly :) | 16:59 |
zzzeek | YorikSar: i dont know that python-memcached had that threadlocal when i first used it | 16:59 |
*** tedross has joined #openstack-oslo | 17:00 | |
morganfainberg | yay extra thread localing. | 17:00 |
morganfainberg | zzzeek, so i heard you liked thread local... </bad meme reference> | 17:00 |
*** viktors is now known as viktors|afk | 17:01 | |
YorikSar | zzzeek: I got a bug "Keystone doesn't reuse connections to memcached" (leads to thousands of connections and "Too many open files"). I went to dogpile.cache and said "Ah, unavoidable threadlocal here...", started looking deeper and found python-memcached... Citing recent xkcd post: turtles all the way down | 17:01 |
morganfainberg | YorikSar, ++ | 17:01 |
morganfainberg | does pylibmc or bmemcached do the samething? | 17:02 |
zzzeek | re: dogpile patches, if we are using these things in load test envrionemnts, that helps a lot | 17:02 |
zzzeek | morganfainberg: pylibmc has an optional pool. but its in C and probbly doesnt support eventlet patching | 17:02 |
morganfainberg | ick | 17:02 |
morganfainberg | so we'd need explicit time.sleep(0) support | 17:02 |
morganfainberg | for yielding. | 17:02 |
* dhellmann crosses his fingers again for the oslotest release... | 17:03 | |
YorikSar | zzzeek: btw, my patch will be released in Mirantis OpenStack, so it can get some production use even before I propose it to dogpile.cache :) | 17:03 |
zzzeek | YorikSar: OK | 17:03 |
morganfainberg | you know. I'm tempted to say maybe it's time to just re-write / new impl of non-silly memcache python lib. | 17:03 |
zzzeek | morganfainberg: +1 | 17:03 |
*** i159 has quit IRC | 17:03 | |
YorikSar | morganfainberg: There already is one :) | 17:03 |
morganfainberg | zzzeek, it's not a complex protocol | 17:04 |
YorikSar | memcached_client | 17:04 |
zzzeek | morganfainberg: its not | 17:04 |
morganfainberg | YorikSar, is it something actively developed? | 17:04 |
morganfainberg | YorikSar and/or widely available | 17:04 |
*** openstackgerrit has quit IRC | 17:04 | |
morganfainberg | YorikSar, doesn't look like it's on pip and not well distributed. | 17:05 |
* YorikSar looking for it... | 17:05 | |
YorikSar | Here it is: https://github.com/mixpanel/memcache_client | 17:05 |
morganfainberg | yeah | 17:05 |
*** harlowja_away is now known as harlowja | 17:05 | |
morganfainberg | toabctl, doesn't look very active | 17:05 |
morganfainberg | not toabctl, YorikSar | 17:06 |
YorikSar | It doesn't support much, but from the blog post with rationale I gather they had almost the same problems: eventlet and bad-bad python-memcached (swallowing errors). | 17:06 |
morganfainberg | yeah | 17:06 |
YorikSar | morganfainberg: Yep, It's not very active. But we can make it one ;) | 17:06 |
morganfainberg | YorikSar, ++ or we fork / stackforge it get it on pip | 17:07 |
morganfainberg | but that isn't a juno target now. that'd be Kilo at best. | 17:07 |
YorikSar | morganfainberg: But on the other hand, python-memcached isn't that bad for our case, actually... Just threadlocal stuff bites our green threads. | 17:07 |
morganfainberg | we'd need some mokey-patching for juno / icehouse | 17:07 |
morganfainberg | YorikSar, yeah not sold we should re-impl python-memcached but not sold we're not going have issues with python-memcached long term still. | 17:08 |
morganfainberg | YorikSar, i think we should work on a monkeypatch / local fix and then do the gut check on where we are for getting python-memcached fixed. | 17:09 |
ekarlso- | zzzeek: around for a sqla core q ? | 17:09 |
zzzeek | yeah | 17:09 |
YorikSar | morganfainberg: python-memcached has way more functionality in. E.g. dogpile.cache won't even work with memcache_client since it doesn't have 'add' support. | 17:09 |
morganfainberg | YorikSar, i mean if we make memcached_client the target we would need to make it full featured. | 17:09 |
ekarlso- | zzzeek: http://paste.openstack.org/show/106653/ | 17:10 |
ekarlso- | ignore the ipdb | 17:10 |
ekarlso- | but how can I have it return a id if there's no pkey ? | 17:10 |
ekarlso- | tried the returning thing but I think it fails with sqlite | 17:10 |
morganfainberg | YorikSar, or fork/re-impl | 17:10 |
zzzeek | ekarlso-: sqlite doesnt support RETURNING (nor does MySQL ...) | 17:10 |
*** markmcclain1 has quit IRC | 17:10 | |
ekarlso- | zzzeek: so if no pkey you're bork ? | 17:11 |
zzzeek | ekarlso-: core gives you result.last_inseted_pk | 17:11 |
ekarlso- | zzzeek: is that "safe" ? | 17:11 |
zzzeek | ekarlso-: let me get the name | 17:11 |
ekarlso- | zzzeek: uhm, but problem | 17:11 |
zzzeek | ekarlso-: result.inserted_primary_key | 17:11 |
ekarlso- | the row doesn't have a pk | 17:11 |
ekarlso- | ehm, table | 17:11 |
zzzeek | ekarlso-: that is the PK of the table as inserted | 17:11 |
*** HenryG is now known as HenryThe8th | 17:12 | |
zzzeek | ekarlso-: dont use returning() if you want to call inserted_primary_key | 17:12 |
YorikSar | morganfainberg, zzzeek: Here's my code that went to MOS: https://gist.github.com/YorikSar/290fe236aca4713ad785 - it's been made as non-intrusive as it could, so for real work it'll need some work on dogpile.cache's GenericBackend and python-memcached side. | 17:12 |
*** ildikov_ has joined #openstack-oslo | 17:12 | |
ekarlso- | zzzeek: but as I said, the table doesn't have a pkey :D | 17:12 |
zzzeek | ekarlso-: then you have no way to locate distinct rows within it | 17:12 |
zzzeek | ekarlso-: wont work anywhere | 17:13 |
morganfainberg | YorikSar, and for icehouse and juno we won't be able to rely on that unless we pull that into keystone | 17:13 |
zzzeek | YorikSar: background thread, huh ? | 17:13 |
morganfainberg | YorikSar, because requirements. | 17:13 |
YorikSar | morganfainberg: Yep. So only distrib-specific patches are possible here. | 17:14 |
YorikSar | zzzeek: We can make it very optional. But it's really useful for production - purging unused connections. | 17:14 |
*** ildikov_ has quit IRC | 17:14 | |
YorikSar | zzzeek: And it won't live longer that pool object. | 17:15 |
zzzeek | YorikSar: OK…this is a lot of heavy stuff to put straight into dogpile | 17:15 |
zzzeek | YorikSar: Im +1 for pluggable pools | 17:15 |
*** ildikov has quit IRC | 17:15 | |
morganfainberg | zzzeek, i agree, this is a lot of stuff to add. but pluggable pools would be nice. | 17:15 |
YorikSar | zzzeek: Yeah, I actually thought about doing a separate lib that would do proper yet abstract connection pooling. | 17:16 |
zzzeek | morganfainberg: pluggable pools yes. Some basic pools yes. the background thing, eh maybe. it pushes people towards certain implementations that might not be what’s appropriate for them | 17:16 |
YorikSar | zzzeek: I didn't find one on PyPI (only very constrained twitter one). | 17:16 |
morganfainberg | zzzeek, ++ | 17:16 |
zzzeek | YorikSar: I’d say, a pluggable pool system and a basic pool in dogpile and we can link out to a more “this is the heavyweight one we use in production” with the backgrounding and all that on pypi | 17:17 |
morganfainberg | zzzeek, big +1 to that | 17:17 |
YorikSar | zzzeek: And then we'll need another pack of setuptools endpoints for pools. | 17:17 |
morganfainberg | YorikSar, not the end of the world | 17:18 |
zzzeek | YorikSar: b.c. this pool is pretty intense with the notify_all() and all that :) | 17:18 |
dstanek | morganfainberg: at one time there was a spec for this - https://blueprints.launchpad.net/oslo/+spec/eventlet-safe-memcache-client | 17:18 |
dstanek | seems to be gone now | 17:18 |
zzzeek | YorikSar: you want the pools via endpoints? OK….you can get by without that…either way... | 17:18 |
morganfainberg | dstanek, we *might* just jump on that for Kilo | 17:18 |
YorikSar | zzzeek: btw, if we have it in a separate lib, dogpile will still have to specialize it for memcached... | 17:18 |
zzzeek | YorikSar: yes. i just want the fancy wait/release/acquisiton count/backgrounding thread not to be in dogpile.cache proper, its too much code to maintain | 17:19 |
zzzeek | YorikSar: this kind of stuff breaks a lot | 17:19 |
zzzeek | YorikSar: when someone else uses it ina different way than you do | 17:19 |
zzzeek | YorikSar: sqlalchemy conneciton pool has taken years and years | 17:19 |
zzzeek | YorikSar: and is not quite as intense as this :) | 17:19 |
YorikSar | zzzeek: Hey, my code is unbreakable :) | 17:19 |
zzzeek | YorikSar: yup | 17:19 |
YorikSar | zzzeek: I totally understand. Btw, I think even threadlocal 'pool' might be one of the options for that lib. | 17:20 |
morganfainberg | YorikSar, right until we break it :P *oops* | 17:20 |
YorikSar | zzzeek: From what I understand sqlalchemy has very specialized pool, isn't it? | 17:22 |
zzzeek | YorikSar: it is | 17:22 |
YorikSar | zzzeek: Or we can cut some of its pieces out and reuse for memcached?.. | 17:22 |
* dhellmann disappears for lunch | 17:22 | |
zzzeek | YorikSar: im not really sure. it started with a goofy approach years ago | 17:22 |
*** dhellmann is now known as dhellmann_ | 17:23 | |
YorikSar | zzzeek: I think it worth investigating. | 17:23 |
zzzeek | YorikSar: feel free :) | 17:23 |
YorikSar | zzzeek: Yeah. I think I will look into it. | 17:23 |
morganfainberg | ok so. hm. | 17:24 |
morganfainberg | forward looking we have a lot of options | 17:24 |
morganfainberg | now... gotta solve Juno and Icehouse. is monkey patch going to really be the only way? | 17:24 |
YorikSar | morganfainberg: I guess fixing those will have to wait until libraries get ready... | 17:26 |
morganfainberg | yeah | 17:26 |
morganfainberg | well we can't add new dependencies | 17:26 |
morganfainberg | is the bigger issue | 17:26 |
morganfainberg | and we *do* recommend apache deployment over eventlet, but we have to support eventlet | 17:27 |
YorikSar | morganfainberg: I think it will be another recomendation: install this lib as well and set up dogpile that way. | 17:27 |
morganfainberg | YorikSar, sure, that doesn't really fly in a lot of orgs. | 17:29 |
morganfainberg | YorikSar they limit the dependencies. | 17:29 |
YorikSar | morganfainberg: Well... It seems to be up to distro maintainer. | 17:30 |
morganfainberg | YorikSar, that and the deployers | 17:30 |
YorikSar | morganfainberg: Yeah... | 17:30 |
morganfainberg | YorikSar, my bigger concern is deployers say we ned requirements <list> | 17:31 |
morganfainberg | and if you say "oh to solve this use this other thing" they wont because it's not certified as ok | 17:31 |
morganfainberg | until upgrade to the next version of OS that really depends on it | 17:31 |
morganfainberg | s/OS/openstack | 17:31 |
YorikSar | morganfainberg: Well... Doesn't upgrade of minor version count? | 17:32 |
morganfainberg | YorikSar, no not for new dependencies | 17:32 |
YorikSar | morganfainberg: Then they'll have to patch Keystone as we did... | 17:32 |
morganfainberg | YorikSar again, you're asking someone to expand the requirements on a "stable" release | 17:32 |
morganfainberg | YorikSar, right the thought is we need that patch for keystone in Juno and backported to icehouse | 17:33 |
morganfainberg | and in K we can leverage the new shiney working libs | 17:33 |
YorikSar | morganfainberg: Do you think code like this plus some duct-taping in kvs has a chance to land in stable branch? | 17:34 |
morganfainberg | if we're not adding new config options i think we might be able to | 17:34 |
morganfainberg | this is a nasty bug. | 17:34 |
morganfainberg | if the code *really* only fixes the bug as tightly as we can address it, it should be fine for stable | 17:35 |
*** AAzza is now known as AAzza_afk | 17:35 | |
YorikSar | morganfainberg: It will still require some config changes - user has to select this backend in [cache] section. | 17:35 |
morganfainberg | that is a choice for the deployer, and for the token memcache backend we control what dogpile backend is used https://github.com/openstack/keystone/blob/master/keystone/token/persistence/backends/memcache.py#L24 | 17:37 |
dims | dhellmann_: not sure if you saw that i got pbr+sphinx auto-generate to hide private and common modules | 17:37 |
morganfainberg | YorikSar, we're not needing to change any config options in keystone common.config | 17:37 |
morganfainberg | and actually | 17:37 |
YorikSar | morganfainberg: It actually fixes 2 bugs: https://bugs.launchpad.net/keystone/+bug/1360446 and https://bugs.launchpad.net/keystone/+bug/1332058 | 17:37 |
morganfainberg | we control somewhat the backend for memcache in kvs anyway | 17:38 |
morganfainberg | https://github.com/openstack/keystone/blob/master/keystone/common/kvs/__init__.py#L30-L33 | 17:38 |
YorikSar | morganfainberg: Yep, I patch that too... | 17:38 |
morganfainberg | we can do some magic there | 17:38 |
morganfainberg | in the manager that wraps pylibmc/bmemcached/etc | 17:38 |
YorikSar | morganfainberg: Ok, I got to go now. I guess I'll push this change to upstream Gerrit and we'll see | 17:40 |
morganfainberg | thanks. | 17:40 |
YorikSar | how it can be landed... | 17:40 |
* morganfainberg is on stable-maint so can help champion that. | 17:40 | |
YorikSar | Since it doesn't look like something I'd like to keep around in master... | 17:40 |
morganfainberg | well we will need it for juno still :( | 17:40 |
morganfainberg | but in K we can rip it out for better approaches / new dependencies | 17:41 |
morganfainberg | i don't like it, but it is a real probelm | 17:41 |
* YorikSar gets ready to propose patch to stable/juno before it exists | 17:41 | |
morganfainberg | oh you *may* want to make the patching logic in the keystone.common.environment.use_eventlet function | 17:41 |
morganfainberg | so we only patch when we use eventlet | 17:41 |
YorikSar | morganfainberg: Ok, I'll create a CR later today then. See you soon. | 17:41 |
morganfainberg | YorikSar, if needed i can help with that. | 17:42 |
morganfainberg | YorikSar, cheers and thanks! | 17:42 |
morganfainberg | dstanek, ^ info on moving on that stuff for memcache ick | 17:42 |
harlowja | dims do u mind re +2ing https://review.openstack.org/#/c/117424/ and https://review.openstack.org/#/c/118731/ (i had to update them due to a requirements file split that went in before these and caused these two to need a rebase) | 17:46 |
dims | harlowja: done | 17:47 |
harlowja | thx | 17:47 |
*** AAzza_afk is now known as AAzza | 17:52 | |
*** jaypipes has joined #openstack-oslo | 17:53 | |
*** dhellmann_ is now known as dhellmann | 18:02 | |
*** jgrimm is now known as zz_jgrimm | 18:09 | |
*** jaypipes is now known as leakypipes | 18:09 | |
*** openstackgerrit has joined #openstack-oslo | 18:10 | |
r1chardj0n3s | can anyone explain how oslo/middleware/openstack/common is managed please? | 18:37 |
openstackgerrit | A change was merged to openstack/oslotest: warn against sorting requirements https://review.openstack.org/118734 | 18:38 |
openstackgerrit | Joshua Harlow proposed a change to openstack/taskflow: Ensure the cachedproperty attribute creation/setting is thread-safe https://review.openstack.org/119443 | 18:52 |
bknudson | r1chardj0n3s: it's synced from oslo-incubator | 18:52 |
openstackgerrit | Joshua Harlow proposed a change to openstack/taskflow: Ensure the cachedproperty creation/setting is thread-safe https://review.openstack.org/119443 | 18:53 |
*** flaper87 is now known as flaper87|afk | 19:01 | |
*** openstackgerrit has quit IRC | 19:01 | |
*** openstackgerrit has joined #openstack-oslo | 19:02 | |
openstackgerrit | Joshua Harlow proposed a change to openstack/taskflow: Ensure the cachedproperty creation/setting is thread-safe https://review.openstack.org/119443 | 19:02 |
r1chardj0n3s | bknudson: how is the sync done? | 19:05 |
bknudson | r1chardj0n3s: https://wiki.openstack.org/wiki/Oslo#Syncing_Code_from_Incubator | 19:05 |
r1chardj0n3s | Thanks! | 19:06 |
*** stevemar has quit IRC | 19:23 | |
*** stevemar has joined #openstack-oslo | 19:24 | |
*** AAzza is now known as AAzza_afk | 19:40 | |
r1chardj0n3s | dhellmann: looks like that original change (https://review.openstack.org/#/c/21958) wasn't actually compliant :/ | 19:58 |
dhellmann | r1chardj0n3s: with cors you mean? | 19:59 |
r1chardj0n3s | dhellmann: yeah | 19:59 |
r1chardj0n3s | dhellmann: I'm currently re-reading the CORS W3C standard and there's several deviations | 20:00 |
r1chardj0n3s | dhellmann: also, what do I need to do to replace testr with py.test in the test suite? :) | 20:00 |
dhellmann | r1chardj0n3s: sigh | 20:00 |
mtreinish | dhellmann: on https://bugs.launchpad.net/oslo.config/+bug/1365136 I'm pretty sure that's just on the oslo-incubator sample generator | 20:01 |
dhellmann | r1chardj0n3s: start a thread on the ML and get some consensus, I suppose | 20:01 |
dhellmann | mtreinish: that version of the generator is being deprecated | 20:01 |
r1chardj0n3s | dhellmann: I'll put that on my TODO :) | 20:01 |
mtreinish | I was looking at the code for the fancy new one (which is a lot nicer) and it looks like it properly sorts the lists | 20:01 |
dhellmann | mtreinish: so maybe I can close that bug? | 20:01 |
openstackgerrit | Joshua Harlow proposed a change to openstack/taskflow: Remove the dependency on prettytable https://review.openstack.org/117629 | 20:01 |
dhellmann | mtreinish: awesome, closing :-) | 20:01 |
mtreinish | dhellmann: well it's still a tempest bug until we can switch to using the new on in tox -epep8 | 20:01 |
dhellmann | mtreinish: yeah, I was going to mark it wontfix for oslo.config, but maybe I should just remove oslo.config from the project list | 20:02 |
mtreinish | r1chardj0n3s: you're still tilting at that windmill? | 20:02 |
mtreinish | dhellmann: yeah that's probably better, or mark it as invalid for oslo.config | 20:02 |
r1chardj0n3s | mtreinish: I've spent more time fighting testr than actually being productive, so yeah | 20:02 |
dhellmann | r1chardj0n3s: I am not opposed to changing that, but don't really have time to champion the effort right now. | 20:03 |
r1chardj0n3s | dhellmann: ok | 20:03 |
dhellmann | r1chardj0n3s: and I'd like to not have oslo be different, so it would be better if we got some sort of "yes, that's ok" from infra and some other projects before we started making changes | 20:04 |
r1chardj0n3s | dhellmann: roger | 20:04 |
openstackgerrit | Matt Riedemann proposed a change to openstack-dev/hacking: Update H501 to check for self.__dict__ https://review.openstack.org/119460 | 20:08 |
*** _amrith_ is now known as amrith | 20:20 | |
openstackgerrit | Joshua Harlow proposed a change to openstack/taskflow: Add existing types to generated documentation https://review.openstack.org/106930 | 20:42 |
*** miqui has quit IRC | 20:47 | |
*** tedross has quit IRC | 20:55 | |
*** pcm_ has quit IRC | 21:08 | |
*** dims has quit IRC | 21:08 | |
*** dims has joined #openstack-oslo | 21:12 | |
*** jecarey has quit IRC | 21:13 | |
*** leakypipes is now known as jaypipes-afk | 21:14 | |
*** jeckersb is now known as jeckersb_gone | 21:25 | |
openstackgerrit | Richard Jones proposed a change to openstack/oslo-specs: Add CORS middleware spec https://review.openstack.org/119485 | 21:28 |
*** openstackgerrit has quit IRC | 21:31 | |
*** openstackgerrit has joined #openstack-oslo | 21:32 | |
openstackgerrit | Richard Jones proposed a change to openstack/oslo-specs: Add CORS middleware spec https://review.openstack.org/119485 | 21:33 |
*** zzzeek has quit IRC | 21:41 | |
*** alexpilotti has quit IRC | 21:42 | |
*** stannie1 has quit IRC | 21:43 | |
*** dims_ has joined #openstack-oslo | 21:44 | |
openstackgerrit | Valeriy Ponomaryov proposed a change to openstack/oslo.utils: Make safe_encode func case-insensitive https://review.openstack.org/119489 | 21:45 |
*** dims_ has quit IRC | 21:46 | |
*** dims has quit IRC | 21:47 | |
*** dims_ has joined #openstack-oslo | 21:47 | |
*** dims_ has quit IRC | 21:57 | |
*** dims has joined #openstack-oslo | 21:58 | |
*** dims has quit IRC | 22:06 | |
*** kgiusti has left #openstack-oslo | 22:11 | |
*** stevemar has quit IRC | 22:28 | |
*** bknudson has quit IRC | 22:30 | |
openstackgerrit | A change was merged to openstack/oslo.vmware: Work toward Python 3.4 support and testing https://review.openstack.org/118795 | 22:44 |
*** mtanino has quit IRC | 22:58 | |
*** harlowja is now known as harlowja_away | 23:00 | |
*** praneshp has quit IRC | 23:23 | |
*** praneshp has joined #openstack-oslo | 23:26 | |
*** harlowja_away is now known as harlowja | 23:31 | |
*** arnaud__ has joined #openstack-oslo | 23:40 | |
*** jaypipes-afk has quit IRC | 23:40 |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!