*** amotoki_ has joined #openstack-oslo | 00:04 | |
amrith | bnemec, yt? | 00:12 |
---|---|---|
*** ajo has quit IRC | 00:18 | |
*** tsekiyama has quit IRC | 00:19 | |
*** gangil has quit IRC | 00:22 | |
*** ajo has joined #openstack-oslo | 00:23 | |
*** celttechie has quit IRC | 00:25 | |
amrith | bnemec, if you see this, please take a look at https://review.openstack.org/#/c/109469/2 ... posted a couple of questions to you on your review. Please let me know what you think; I'm either on here or on #openstack-trove. Or at amrith@tesora.com. Thx! | 00:26 |
*** amrith is now known as amrith_ | 00:27 | |
*** amrith_ is now known as amrith | 00:27 | |
*** tsekiyama has joined #openstack-oslo | 00:33 | |
*** stevemar is now known as maybeiamstevemar | 00:35 | |
*** shakamunyi has quit IRC | 00:43 | |
*** tsekiyama has quit IRC | 00:45 | |
*** praneshp has joined #openstack-oslo | 01:01 | |
*** celttechie has joined #openstack-oslo | 01:26 | |
*** tsekiyama has joined #openstack-oslo | 01:32 | |
*** morganfainberg_Z is now known as morganfainberg | 01:35 | |
openstackgerrit | A change was merged to openstack/oslo-incubator: Remove usage of readlines() https://review.openstack.org/110228 | 01:52 |
*** shakamunyi has joined #openstack-oslo | 01:54 | |
*** tsekiyama has quit IRC | 01:56 | |
*** tsekiyama has joined #openstack-oslo | 01:59 | |
openstackgerrit | A change was merged to openstack-dev/oslo-cookiecutter: Add test script https://review.openstack.org/110000 | 02:01 |
*** tsekiyama has quit IRC | 02:03 | |
*** shakamunyi has quit IRC | 02:07 | |
*** shakamunyi has joined #openstack-oslo | 02:22 | |
*** HenryG is now known as HenryG_afk | 02:27 | |
*** yamahata has joined #openstack-oslo | 02:28 | |
openstackgerrit | Zhongyue Luo proposed a change to openstack/oslo.config: Log a fixed length string of asterisks for obfuscation https://review.openstack.org/111888 | 02:40 |
openstackgerrit | A change was merged to openstack/cliff: Clean up default tox environment list https://review.openstack.org/109582 | 02:59 |
*** arnaud has quit IRC | 03:11 | |
*** SridharG has joined #openstack-oslo | 04:24 | |
*** arnaud has joined #openstack-oslo | 04:30 | |
*** k4n0 has joined #openstack-oslo | 04:43 | |
*** morganfainberg is now known as morganfainberg_Z | 04:54 | |
*** jaosorior has joined #openstack-oslo | 05:18 | |
*** celttechie has quit IRC | 05:23 | |
openstackgerrit | Steve Martinelli proposed a change to openstack/oslo-incubator: Use oslosphinx to generate documentation https://review.openstack.org/111910 | 05:30 |
*** shakamunyi has quit IRC | 05:42 | |
*** flaper87|afk is now known as flaper87 | 05:56 | |
openstackgerrit | OpenStack Proposal Bot proposed a change to openstack/oslo.db: Imported Translations from Transifex https://review.openstack.org/111918 | 06:03 |
*** shakamunyi has joined #openstack-oslo | 06:09 | |
openstackgerrit | OpenStack Proposal Bot proposed a change to openstack/oslo.utils: Imported Translations from Transifex https://review.openstack.org/111166 | 06:11 |
*** shakamunyi has quit IRC | 06:24 | |
*** Alexei_987 has quit IRC | 06:31 | |
*** ildikov has joined #openstack-oslo | 06:32 | |
*** i159 has joined #openstack-oslo | 06:35 | |
*** flaper87 is now known as flaper87|afk | 06:41 | |
*** shakamunyi has joined #openstack-oslo | 06:50 | |
*** pblaho has joined #openstack-oslo | 06:52 | |
*** shakamunyi has quit IRC | 06:58 | |
*** ihrachyshka has joined #openstack-oslo | 07:09 | |
*** ihrachyshka has quit IRC | 07:13 | |
*** ihrachyshka has joined #openstack-oslo | 07:14 | |
*** abhishek has joined #openstack-oslo | 07:23 | |
*** praneshp_ has joined #openstack-oslo | 07:28 | |
*** praneshp has quit IRC | 07:30 | |
*** praneshp_ is now known as praneshp | 07:30 | |
*** praneshp has quit IRC | 07:41 | |
*** AAzza_afk is now known as AAzza | 07:48 | |
*** ihrachyshka has quit IRC | 07:53 | |
*** ihrachyshka has joined #openstack-oslo | 07:53 | |
*** shakamunyi has joined #openstack-oslo | 07:55 | |
*** arnaud has quit IRC | 08:00 | |
*** maybeiamstevemar has quit IRC | 08:06 | |
*** alexpilotti has joined #openstack-oslo | 08:07 | |
*** shakamunyi has quit IRC | 08:10 | |
*** tsufiev has quit IRC | 08:40 | |
*** tsufiev has joined #openstack-oslo | 08:45 | |
*** andreykurilin1 has quit IRC | 08:47 | |
*** Alexei_987 has joined #openstack-oslo | 09:00 | |
*** oomichi has quit IRC | 09:16 | |
tsufiev | hi there! could you please take a look at https://review.openstack.org/#/c/104916/ ? | 09:36 |
ihrachyshka | dhellmann_: can I ask you to reconsider your vote on: https://review.openstack.org/#/c/91457/ ? | 09:46 |
ihrachyshka | it's for stable/havana | 09:46 |
*** pblaho is now known as pblaho|lunch | 09:53 | |
*** shakamunyi has joined #openstack-oslo | 09:57 | |
rpodolyaka | tsufiev: done | 09:58 |
*** shakamunyi has quit IRC | 10:01 | |
*** flaper87|afk is now known as flaper87 | 10:01 | |
tsufiev | rpodolyaka, thank you, will address your comments in a moment | 10:02 |
*** flaper87 is now known as flaper87|afk | 10:16 | |
*** yamahata has quit IRC | 10:21 | |
ajo | dhellmann_, thanks for the comments here, https://review.openstack.org/#/c/97748/ I added some more comments, if you have some time take a second look to make sure what I say makes sense. | 10:26 |
*** AAzza is now known as AAzza_afk | 10:27 | |
openstackgerrit | A change was merged to openstack/oslo.db: Imported Translations from Transifex https://review.openstack.org/111918 | 10:36 |
openstackgerrit | Yuriy Taraday proposed a change to openstack/oslo-incubator: Switch to using SysV semaphores to allow safe process termination https://review.openstack.org/108954 | 10:52 |
*** viktors has joined #openstack-oslo | 10:55 | |
*** pblaho|lunch is now known as pblaho | 10:58 | |
*** AAzza_afk is now known as AAzza | 11:10 | |
*** amotoki_ has quit IRC | 11:11 | |
*** flaper87|afk is now known as flaper87 | 11:33 | |
*** shakamunyi has joined #openstack-oslo | 11:47 | |
*** HenryG_afk has quit IRC | 11:50 | |
*** flaper87 is now known as flaper87|afk | 11:54 | |
*** shakamunyi has quit IRC | 11:58 | |
*** penguinRaider__ has quit IRC | 12:08 | |
*** penguinRaider__ has joined #openstack-oslo | 12:10 | |
*** SridharG has quit IRC | 12:12 | |
openstackgerrit | Ilya Pekelny proposed a change to openstack/oslo.db: Implementation of Alembic as migration engine https://review.openstack.org/99965 | 12:12 |
*** penguinRaider__ has quit IRC | 12:28 | |
*** penguinRaider__ has joined #openstack-oslo | 12:29 | |
*** flaper87|afk is now known as flaper87 | 12:34 | |
*** ochuprykov has joined #openstack-oslo | 12:35 | |
*** tongli has joined #openstack-oslo | 12:36 | |
*** SridharG has joined #openstack-oslo | 12:39 | |
*** penguinRaider__ has quit IRC | 12:48 | |
*** penguinRaider__ has joined #openstack-oslo | 12:49 | |
*** k4n0 has quit IRC | 12:50 | |
*** YorikSar has joined #openstack-oslo | 12:56 | |
*** penguinRaider__ has quit IRC | 12:58 | |
*** jecarey has quit IRC | 12:58 | |
*** penguinRaider__ has joined #openstack-oslo | 13:00 | |
*** markmcclain has joined #openstack-oslo | 13:02 | |
*** gordc has joined #openstack-oslo | 13:02 | |
*** yamahata has joined #openstack-oslo | 13:02 | |
*** pblaho has quit IRC | 13:05 | |
*** pblaho_ has joined #openstack-oslo | 13:05 | |
*** ttx has quit IRC | 13:06 | |
amrith | YorikSar, yt? | 13:09 |
YorikSar | amrith: Hi, yes | 13:09 |
amrith | g'morning Yuriy. figured it may be easier to discuss on IRC than review. | 13:09 |
amrith | what should I be looking for in the errant monkey_patch? | 13:09 |
YorikSar | amrith: Yeah... | 13:09 |
amrith | in testing we import eventlet | 13:10 |
amrith | and immediately monkey_patch | 13:10 |
amrith | i.e. | 13:10 |
amrith | import eventlet | 13:10 |
amrith | eventlet.monkey_patch(thread=False) | 13:10 |
YorikSar | Ok... That's strange. | 13:10 |
YorikSar | It seems os should've been monkey-patched. | 13:11 |
YorikSar | But os.read raises EAGAIN => it's not monkey-patched. | 13:11 |
amrith | this appears to be what we're doing. | 13:11 |
amrith | http://eventlet.net/doc/patching.html | 13:11 |
YorikSar | btw why is thread excluded?.. That's unusual for me. | 13:12 |
amrith | that beats me | 13:12 |
*** mgagne has quit IRC | 13:12 | |
amrith | to be clear | 13:12 |
amrith | I'm not sure what that does | 13:12 |
amrith | whether it means not to patch thread calls | 13:12 |
*** dhellmann_ is now known as dhellmann | 13:12 | |
amrith | or something else | 13:13 |
*** ttx has joined #openstack-oslo | 13:13 | |
*** ttx has quit IRC | 13:13 | |
*** ttx has joined #openstack-oslo | 13:13 | |
*** mgagne has joined #openstack-oslo | 13:13 | |
*** mgagne is now known as Guest27295 | 13:13 | |
amrith | what would you expect os.read to be patched to? there's no read () that I can find in eventlet | 13:14 |
YorikSar | amrith: Yes, it means that threads will not be greened. | 13:14 |
YorikSar | Along with everything that's thread-related (locks and all that stuff). | 13:14 |
*** gordc has quit IRC | 13:14 | |
*** abhishek has quit IRC | 13:14 | |
*** jaypipes has quit IRC | 13:14 | |
*** jokke_ has quit IRC | 13:14 | |
*** morganfainberg_Z has quit IRC | 13:14 | |
amrith | so the only read() in eventlet is in wsgi. | 13:15 |
YorikSar | amrith: Here it is: https://bitbucket.org/eventlet/eventlet/src/tip/eventlet/green/os.py#cl-29 | 13:15 |
amrith | that's not going to be something you want to patch to os.read, I don't think. | 13:15 |
* amrith looks | 13:15 | |
*** gordc has joined #openstack-oslo | 13:15 | |
*** abhishek has joined #openstack-oslo | 13:15 | |
*** jaypipes has joined #openstack-oslo | 13:15 | |
*** jokke_ has joined #openstack-oslo | 13:15 | |
*** morganfainberg_Z has joined #openstack-oslo | 13:15 | |
YorikSar | It does the same thing all other greened calls do: retry syscall if EAGAIN is caught. | 13:16 |
ihrachyshka | amrith: https://github.com/eventlet/eventlet/blob/master/eventlet/green/os.py#L29 | 13:17 |
*** lbragstad has quit IRC | 13:18 | |
*** jecarey has joined #openstack-oslo | 13:18 | |
amrith | ihrachyshka, thx. | 13:18 |
amrith | looks like the same one that YorikSar sent | 13:18 |
YorikSar | The strange thing is that subprocess appears to be patched since os.read on its pipe returns EAGAIN. But os is not patched since os.read passes it through. | 13:19 |
YorikSar | ihrachyshka, amrith: Some people just don't like bitbucket ;) | 13:19 |
amrith | and clearly close() appears to be patched | 13:19 |
amrith | because it is getting to _fileoperationonclosedfile() or whatever it is called | 13:19 |
*** ttx has quit IRC | 13:20 | |
ihrachyshka | YorikSar: ;) | 13:22 |
YorikSar | amrith: I didn't see such method in traces.. Did I miss smth? | 13:22 |
*** lbragstad has joined #openstack-oslo | 13:23 | |
amrith | let me get it to you | 13:23 |
amrith | one second | 13:23 |
ihrachyshka | YorikSar: so maybe this is due to monkey_patch() called not at the very start? so some modules already imported the original version and use it internally? | 13:23 |
amrith | _operationOnClosedFile | 13:24 |
*** sheeprine has joined #openstack-oslo | 13:24 | |
sheeprine | Hi, I've got a little question about oslo.db and the alembic extension for migration_cli. Is it me or the version() method is broken? It's not using current() method from alembic and MigrationContext doesn't get the alembic config, in my case it's a show stopper. | 13:24 |
*** ttx has joined #openstack-oslo | 13:25 | |
*** ttx has quit IRC | 13:25 | |
*** ttx has joined #openstack-oslo | 13:25 | |
dhellmann | rpodolyaka, viktors : are you online? | 13:25 |
*** mriedem has joined #openstack-oslo | 13:25 | |
*** mriedem has quit IRC | 13:25 | |
*** mriedem has joined #openstack-oslo | 13:25 | |
viktors | dhellmann: yes | 13:26 |
rpodolyaka | dhellmann: here | 13:26 |
dhellmann | viktors, rpodolyaka : ^^ question from sheeprine | 13:27 |
YorikSar | ihrachyshka: Yes, it seems so. | 13:27 |
viktors | dhellmann, sheeprine: ok, reading | 13:27 |
ihrachyshka | YorikSar: so just make sure you call monkey_patch() at the very top of whatever-contains-main() | 13:28 |
amrith | sorry YorikSar ihrachyshka ... how could that me? clearly os.close() is patched ;) | 13:28 |
viktors | sheeprine: yes, this feature is broken at the moment :( | 13:29 |
rpodolyaka | sheeprine: alembic extension for migration_cli is something that is currently broken :( I wish we didn't expose it to the end user in the first place until it's really ready | 13:29 |
ihrachyshka | amrith: well, I don't know the full story, I just mention that if you see that some of your modules see patched stdlib while other don't, it may mean just that | 13:30 |
amrith | oh crap | 13:30 |
rpodolyaka | sheeprine: long story short: we started working on the feature in oslo.incubator last year, and then we had a few discussion on the approach taken. the outcome was: we had to use alembic private API to make this work | 13:30 |
amrith | please take a look at http://eventlet.net/doc/basic_usage.html | 13:30 |
amrith | ihrachyshka, YorikSar ^^ | 13:30 |
YorikSar | amrith: not exactly... flush() is called on stdin object, it's not taken from os module. | 13:31 |
rpodolyaka | sheeprine: we didn't want to do that, so now this is broken and we might even delete it from the tree until it's ready | 13:31 |
amrith | YorikSar, the backtrace shows that we fail in File "/opt/stack/trove/.tox/py27/local/lib/python2.7/site-packages/eventlet/greenio.py", line 419, in _operationOnClosedFile | 13:31 |
amrith | raise ValueError("I/O operation on closed file") | 13:31 |
amrith | but, that aside. | 13:31 |
amrith | how about this? | 13:31 |
rpodolyaka | sheeprine: zzzeek has been investigating how we can make alembic/oslo.db work together nicely | 13:32 |
rpodolyaka | sheeprine: so it's kind of WIP | 13:32 |
ihrachyshka | amrith: that link... what specifically to check? | 13:32 |
amrith | that documentation conflicts with http://eventlet.net/doc/patching.html#monkey-patch | 13:32 |
amrith | about the arguments to monkey_patch | 13:32 |
*** dhellmann has quit IRC | 13:32 | |
amrith | one indicates that defaults are false | 13:32 |
amrith | the other that defaults are None | 13:32 |
*** pblaho__ has joined #openstack-oslo | 13:33 | |
*** dhellmann has joined #openstack-oslo | 13:33 | |
amrith | so maybe the monkey patch isn't doing what we think it is? | 13:33 |
YorikSar | amrith: Yes. But subprocess does "self.stdin.flush()" here by EAGAIN is raised by "os.read(self.stdout.fileno(), smth)" | 13:33 |
ihrachyshka | amrith: maybe. better check code | 13:33 |
YorikSar | amrith: False and None are the same | 13:33 |
amrith | YorikSar, ... the implication was that if the value is None then it is patched | 13:34 |
amrith | see http://eventlet.net/doc/patching.html#monkey-patch | 13:34 |
amrith | default None ... | 13:34 |
amrith | if no parameters are specified everything is to be patched | 13:34 |
ihrachyshka | YorikSar: hm? it mentions all=True in one of the links but not other one | 13:34 |
amrith | yes | 13:34 |
amrith | but YorikSar has another point ... | 13:34 |
YorikSar | amrith, ihrachyshka: Oh, that one. | 13:34 |
amrith | self.stdin() is patched | 13:35 |
amrith | self.stdin.flush() is patched | 13:35 |
amrith | os.read() is not | 13:35 |
amrith | so trove's test code looks like | 13:35 |
amrith | [...] | 13:35 |
amrith | import os | 13:35 |
ihrachyshka | well, at that moment I should retract from commenting further since I lack context | 13:35 |
ihrachyshka | :) | 13:35 |
amrith | [...] | 13:35 |
YorikSar | It defaults to True there and everything that's not specified is set to that value. | 13:35 |
sheeprine | rpodolyaka: the fact is I used this to do my migrations on my project guess I did the wrong thing. I'm willing to submit the patch for the version() method so at least this part is working. But if you plan to remove it from the tree I'm doomed. What am I supposed to use in oslo.db to do my migrations? I planned on using alembic, and I can't use the defaut alembic_version tables (so the db_version() method with the hardcoded value won't | 13:35 |
YorikSar | amrith: Yes, self.stdin is set to GreenPipe here: https://bitbucket.org/eventlet/eventlet/src/tip/eventlet/green/subprocess.py#cl-48 | 13:36 |
*** dhellmann has quit IRC | 13:36 | |
amrith | the question then is why is _communicate() calling os.read() which isn't greened? | 13:36 |
amrith | that's the one throwing the exception | 13:37 |
*** pblaho_ has quit IRC | 13:37 | |
YorikSar | amrith: It has to call os.read because it uses select which (I think) doesn't work well with fileobj's buffering etc. | 13:37 |
*** pblaho__ has quit IRC | 13:37 | |
YorikSar | amrith: So subprocess has to use os.read here. | 13:38 |
amrith | but os.read should've been monkey-patched to eventlet.read(), no? | 13:38 |
YorikSar | amrith: Yes, it should've. | 13:38 |
YorikSar | amrith: But it didn't... | 13:38 |
*** dhellmann has joined #openstack-oslo | 13:39 | |
rpodolyaka | sheeprine: right now, I'd recommend to use plain alembic. this is what neutron/ironic do | 13:39 |
rpodolyaka | sheeprine: the table name is configurable in alembic config | 13:39 |
YorikSar | amrith: I don't know how testing in Trove is organized. I fought with some issues while adding eventlet testing to oslo.rootwrap though. | 13:39 |
YorikSar | amrith: I think monkey-patching either happens too late or just doesn't happen for os module. | 13:40 |
amrith | YorikSar, I don't know how to monkeypatch the os module | 13:40 |
ihrachyshka | amrith: os=True? | 13:40 |
amrith | let me try that | 13:40 |
YorikSar | You can try to log eventlet.patcher.is_monkey_patched(os) somewhere around the call to subprocess... | 13:41 |
amrith | I've still not figured out how to get stdout from my code to be visible if I'm using the test framework ;( | 13:41 |
ihrachyshka | for what I see in trove/cmd/guest.py, monkey_patch() is called really late | 13:42 |
*** shakamunyi has joined #openstack-oslo | 13:42 | |
ihrachyshka | it should be right after import eventlet | 13:42 |
amrith | I will go and clean those up; in this case, I don't think that code comes into play | 13:42 |
ihrachyshka | otherwise all those imports above the call will use non-patched modules internally | 13:42 |
ihrachyshka | well, there are multiple patches() in the code, I don't know which one is in charge in your discussion. just sayin' | 13:43 |
ihrachyshka | amrith: you can run tests with testtools.run | 13:43 |
ihrachyshka | amrith: https://wiki.openstack.org/wiki/Testr#Debugging_.28pdb.29_Tests | 13:43 |
amrith | ihrachyshka, I have no idea ... I'll try | 13:44 |
YorikSar | amrith: You can drop the line of interest directly to console like with open('/dev/pts/<YOURNUM>', 'w') as f: f.write(myvar) | 13:44 |
amrith | ok, doing that YorikSar | 13:44 |
YorikSar | ihrachyshka: Actually patching happens in sys.modules directly but sometimes somehow it doesn't help. So yes, monkey_patch should be called as early as possible. | 13:45 |
sheeprine | rpodolyaka: Ok, I'll use plain alembic then, thanks for your answers | 13:45 |
rpodolyaka | sheeprine: np | 13:46 |
ihrachyshka | YorikSar: well, maybe indeed; I had issues with the following code in oslo: https://github.com/openstack/oslo.messaging/blob/master/oslo/messaging/localcontext.py#L26 so it's not the same case | 13:47 |
ihrachyshka | YorikSar: it created a global object before threading was patched, then lots of cute events occurred | 13:47 |
*** shakamunyi has quit IRC | 13:49 | |
YorikSar | ihrachyshka: Yeah, I can imagine :) | 13:51 |
amrith | YorikSar, ihrachyshka re-running tests with some changes to quickly monkey_patch | 13:53 |
amrith | will let you know how it works | 13:53 |
ihrachyshka | amrith: looking fwd to hear | 13:53 |
*** flaper87 is now known as flaper87|afk | 13:57 | |
*** yamahata has quit IRC | 13:58 | |
*** yamahata has joined #openstack-oslo | 13:59 | |
SergeyLukjanov | dhellmann, hey, could you please ack https://review.openstack.org/#/c/111783/1 ? | 13:59 |
dhellmann | SergeyLukjanov: looking | 14:01 |
dhellmann | SergeyLukjanov: +1 | 14:02 |
*** wendar has quit IRC | 14:04 | |
*** wendar has joined #openstack-oslo | 14:05 | |
*** flaper87|afk is now known as flaper87 | 14:06 | |
ihrachyshka | dhellmann: please reconsider your vote for https://review.openstack.org/91457 | 14:08 |
*** wendar has quit IRC | 14:08 | |
*** wendar has joined #openstack-oslo | 14:08 | |
*** shakamunyi has joined #openstack-oslo | 14:11 | |
openstackgerrit | Matt Riedemann proposed a change to openstack/oslo.db: Handle DB2 SmallInteger type for change_deleted_column_type_to_boolean https://review.openstack.org/112030 | 14:11 |
SergeyLukjanov | dhellmann, thx | 14:14 |
*** HenryG_ has joined #openstack-oslo | 14:15 | |
*** HenryG_ is now known as HenryG | 14:17 | |
dhellmann | ihrachyshka: looking | 14:17 |
dhellmann | ihrachyshka: did the change needed for nova make it into stable/havana as well? | 14:19 |
ihrachyshka | dhellmann: hm?.. I don't know how nova is directly related to this. I'm interested in introducing this for neutron. | 14:22 |
dhellmann | ihrachyshka: gcb mentioned that this change breaks nova, and there was a fix in the incubator (see comment from April 30) | 14:23 |
ihrachyshka | dhellmann: this is covered by another patch in the same series: https://review.openstack.org/#/q/status:open+project:openstack/oslo-incubator+branch:stable/havana+topic:simplejson-stable-backport,n,z | 14:24 |
ihrachyshka | specifically, this one: https://review.openstack.org/95753 | 14:24 |
dhellmann | ihrachyshka: shouldn't the fix come before the change that introduces the break? | 14:25 |
dhellmann | it's good they are all there, but maybe the order of the series is backwards? | 14:25 |
ihrachyshka | dhellmann: they are stacked one on one | 14:25 |
dhellmann | yes, but right now we can merge the change to break things before the change to prevent things from breaking | 14:25 |
dhellmann | unless I haven't had enough tea and am reading the stacking backwards | 14:26 |
ihrachyshka | dhellmann: we first introduced the issue with unicode with the patch I've originally linked to, then fixed on top by the patch that gcb mentioned | 14:26 |
ihrachyshka | dhellmann: well, yes, that's why I want to push them in one go | 14:26 |
dhellmann | yes, that's what I'm saying. We don't want to introduce problems and then fix them in stable. We want to introduce things in an order that does not break things. | 14:26 |
ihrachyshka | dhellmann: we could squash them to avoid that possibility, though I don't personally like squashes in stables | 14:26 |
dhellmann | You can't depend on someone not seeing the intermediate broken version, since many deployers build packages automatically | 14:26 |
dhellmann | just change the order of the patch series (rebase and reorder them, then resubmit) | 14:27 |
ihrachyshka | but that's incubator, we may enforce the sync on per-project basis | 14:27 |
ihrachyshka | dhellmann: I can't reorder, I can only squash (they patch the same code) | 14:27 |
dhellmann | ah, ok | 14:27 |
ihrachyshka | or... what | 14:28 |
dhellmann | ok, so I guess if we approve them all together and then sync as one change into the projects it should be ok | 14:28 |
ihrachyshka | s/waht/wait | 14:28 |
ihrachyshka | dhellmann: you're actually correct, it's possible to change their order | 14:29 |
ihrachyshka | dhellmann: will do :) | 14:29 |
dhellmann | \o/ | 14:29 |
dhellmann | :-) | 14:29 |
ihrachyshka | that's the right thing to do even though no one will appreciate that :D | 14:29 |
dhellmann | ihrachyshka: you and I will appreciate it :-) | 14:31 |
ihrachyshka | mmm... these warm fuzzie!.. | 14:32 |
*** shakamunyi has quit IRC | 14:32 | |
dhellmann | :-) | 14:32 |
amrith | ihrachyshka, YorikSar ... moving monkey_patch() higher up in all places that import eventlet seems to have stemmed the occurence of the failure (on my machine at least). it may be a worthwhile candidate fix. I'll push that code up for trove and see how it works out. | 14:33 |
ihrachyshka | dhellmann: rearranged them: https://review.openstack.org/#/q/status:open+project:openstack/oslo-incubator+branch:stable/havana+topic:bug/1314129,n,z | 14:33 |
ihrachyshka | amrith: yay | 14:33 |
ihrachyshka | as I told: you should beat monkey asap, otherwise it makes mess | 14:34 |
amrith | no cruelty to animals ... | 14:34 |
ihrachyshka | :D | 14:34 |
YorikSar | amrith: Great :) | 14:35 |
dhellmann | ihrachyshka: ok, I'll wait for the test jobs to finish and then have another look | 14:36 |
ihrachyshka | dhellmann: ok tnx | 14:40 |
*** ildikov has quit IRC | 14:55 | |
amrith | YorikSar, ihrachyshka ... got a q for either of you ... | 14:56 |
amrith | if you import eventlet | 14:56 |
amrith | is it safe to just blindly call monkey_patch() after that? | 14:56 |
*** markmcclain1 has joined #openstack-oslo | 14:57 | |
amrith | YorikSar, ihrachyshka ^^ | 14:57 |
YorikSar | amrith: As long as you're near the very top of your app. | 14:57 |
*** markmcclain has quit IRC | 14:57 | |
amrith | yes, I'm at the top of the app | 14:57 |
amrith | but there are cases where we | 14:58 |
amrith | import eventlet | 14:58 |
amrith | but don't have an immediate monkey_patch() | 14:58 |
amrith | or a monkey_patch() at all | 14:58 |
amrith | is it meaningful to have an application that imports eventlet but doesn't monkey_patch() | 14:58 |
amrith | I don't think so | 14:58 |
amrith | but I wanted to get second and third opinions. | 14:59 |
YorikSar | amrith: It's actually OK to have such app. | 14:59 |
YorikSar | amrith: And monkey-patching can be defered if needed (like here: http://git.openstack.org/cgit/openstack/nova/tree/nova/cmd/__init__.py ) | 14:59 |
YorikSar | amrith: But to do so you should have a very good reason (like having to analyze some state first) | 15:00 |
amrith | ok, looking at that code | 15:00 |
ihrachyshka | amrith: yes, unless you're sure your code in between doesn't do nasty stuff like it is in the code from oslo.messaging I've posted above, you're better calling it right away | 15:00 |
*** yamahata has quit IRC | 15:04 | |
openstackgerrit | Ihar Hrachyshka proposed a change to openstack/oslo-specs: Switch from MySQLdb to MySQL Connector https://review.openstack.org/108355 | 15:04 |
amrith | so, my change was to aggressively monkey_patch() in each place where eventlet was imported and I'm trying to back off from that and find the place where the monkey_patch() was missed. | 15:05 |
amrith | will keep you posted | 15:05 |
amrith | progress ... (and thanks for your help) | 15:05 |
amrith | YorikSar, ihrachyshka ^^ | 15:05 |
ihrachyshka | amrith: that's actually what I did once for neutron: https://github.com/openstack/neutron/commit/6ca8cb84fd8f703367e1bd8ee1a2f26071116725 | 15:07 |
YorikSar | amrith: Throw a link to your patch in Trove to oslo change request when you're done so that everybody can see it. | 15:07 |
ihrachyshka | YorikSar: + | 15:08 |
amrith | i feel better already ;) | 15:09 |
*** shakamunyi has joined #openstack-oslo | 15:12 | |
openstackgerrit | Miguel Angel Ajo proposed a change to openstack/oslo-specs: Add service-status-interface spec https://review.openstack.org/97748 | 15:12 |
openstackgerrit | Miguel Angel Ajo proposed a change to openstack/oslo-specs: Add service-status-interface spec https://review.openstack.org/97748 | 15:13 |
*** i159 has quit IRC | 15:13 | |
*** abhishek has quit IRC | 15:17 | |
*** ihrachyshka has quit IRC | 15:19 | |
*** YorikSar has quit IRC | 15:20 | |
*** sreshetn1ak has quit IRC | 15:20 | |
*** YorikSar has joined #openstack-oslo | 15:21 | |
*** sreshetnyak has joined #openstack-oslo | 15:21 | |
*** shakamunyi has quit IRC | 15:28 | |
amrith | YorikSar, ihrachyshka ... you'll get a chuckle out of this. I added this code https://gist.github.com/amrith/fa29d74528e0b10df5f8 to the place where we invoke execute() and guess what I got? exactly one invocation in the place where we keep having failures. | 15:32 |
*** YorikSar has quit IRC | 15:35 | |
*** sreshetnyak has quit IRC | 15:35 | |
*** YorikSar has joined #openstack-oslo | 15:35 | |
*** sreshetnyak has joined #openstack-oslo | 15:35 | |
YorikSar | amrith: I suspect it's not intended - it's rather strange to call execute() durint unit tests | 15:36 |
amrith | yup ... | 15:36 |
amrith | there was a change recently to do this ;) | 15:36 |
amrith | and the change was to the way in which we do the chown operation ... | 15:36 |
*** viktors is now known as viktors|afk | 15:40 | |
*** ochuprykov has quit IRC | 15:49 | |
*** AAzza is now known as AAzza_afk | 15:54 | |
noelbk | Alexei_987: thanks for the review. Could you check https://review.openstack.org/#/c/109373 too please? | 15:58 |
*** arnaud has joined #openstack-oslo | 16:00 | |
Alexei_987 | noelbk: Hi I have it on my schedule :).. a bit busy last couple of days trying to fix HA in our cloud.. it doesn't work cause of oslo.messaging :) | 16:04 |
*** stevelle has joined #openstack-oslo | 16:06 | |
amrith | YorikSar, got a second? | 16:12 |
noelbk | Alexei_987: Thanks :) HA problems can take a long time to fix. What's the next step in getting this merged upstream? | 16:12 |
Alexei_987 | noelbk: have no idea.. I'm not a core dev in messaging :( | 16:13 |
Alexei_987 | noelbk: I have a 2 line patch that is waiting for 2 weeks already | 16:13 |
Alexei_987 | noelbk: it's quite hard to get a review from core dev lately | 16:13 |
*** stevemar has joined #openstack-oslo | 16:13 | |
noelbk | Alexei_987: ok, good luck with your HA fixing. | 16:13 |
YorikSar | amrith: Yep | 16:14 |
amrith | something I'm not sure I understand is this. I added the code in a function (operating_system.py) function was called update_owner and it printed me a backtrace | 16:14 |
amrith | it calls execute_with_timeout, which is in trove.common.utils.py | 16:14 |
amrith | I can't seem to get anything to print from there | 16:14 |
amrith | can't even open a file in /tmp | 16:14 |
amrith | not certain how that can happen | 16:15 |
amrith | the code there is literally f = open ( '/tmp/trace-t.txt', 'a+' ) and a f.write ('Hello') | 16:15 |
amrith | doesn't make it! | 16:15 |
YorikSar | amrith: May be this method gets mocked? | 16:16 |
amrith | one second | 16:16 |
amrith | let me check | 16:16 |
*** openstackstatus has quit IRC | 16:17 | |
*** openstack has joined #openstack-oslo | 16:17 | |
*** openstackstatus has joined #openstack-oslo | 16:18 | |
*** ChanServ sets mode: +v openstackstatus | 16:18 | |
amrith | yup, verified again. can't see anything get out to /tmp file from execute() | 16:20 |
amrith | strange. YorikSar ^^. will keep looking | 16:22 |
YorikSar | amrith: Hm... That's strange... You can try to run it with testtools and add a pdb.set_trace inside (or outside) | 16:22 |
dhellmann | YorikSar: did you see the comments about the local module on https://review.openstack.org/#/c/110070/ ? | 16:26 |
YorikSar | dhellmann: Hi. Yes. I guess I'll abandon that change then. | 16:27 |
dhellmann | YorikSar: yeah, I think that's the right thing, now that jd__ pointed it out | 16:28 |
YorikSar | dhellmann: Btw, it seems oslo.messaging has essentially the same 'local' module oslo.log can end up with: http://git.openstack.org/cgit/openstack/oslo.messaging/tree/oslo/messaging/localcontext.py | 16:30 |
* dhellmann hangs head | 16:31 | |
*** gangil has joined #openstack-oslo | 16:31 | |
YorikSar | dhellmann: Maybe oslo.log should do conditional import of that one?.. | 16:31 |
YorikSar | dhellmann: Although they don't do weak references there (I don't think they're needed) | 16:32 |
dhellmann | YorikSar: yeah, but they could be used, for consistency, I guess | 16:34 |
dhellmann | YorikSar: we don't really want the libraries "guessing" by doing conditional imports | 16:35 |
YorikSar | dhellmann: Well... This feature can be described as "oslo.log will attach your context set in oslo.messaging if it's available" | 16:36 |
dhellmann | YorikSar: it's the "if it's availble" part I don't like, though. We should be able to be explicit about requirements like this. | 16:37 |
YorikSar | dhellmann: Why don't you like it? It's Python dynamism :) | 16:38 |
dhellmann | YorikSar: as it is now, the context being stored by oslo.messaging won't be used by oslo.log, because that code doesn't know to look in the non-standard place to find it | 16:38 |
dhellmann | YorikSar: so I'm not sure why that's being done at all | 16:39 |
YorikSar | dhellmann: Well... I think noone cares too much about contexts in logs anymore (since they just work) and so when Nova switched to oslo.messaging noone noticed that they're gone. | 16:40 |
dhellmann | YorikSar: unless localcontext is part of the public api of oslo.messaging now | 16:40 |
dhellmann | yeah, the localcontext functions are part of the messaging API | 16:40 |
dhellmann | YorikSar: I restored your change for now. I would like one of the rpc experts to help figure out what's going on with the 2 local modules before we rush to a decision | 16:44 |
YorikSar | dhellmann: Well... Do you think that oslo.concurrency might end up a new home for local after all? It seems unlikely to me. | 16:45 |
dhellmann | YorikSar: no, but the original plan was to have it in its own library, and I think this may have been why. | 16:46 |
* dhellmann needs to keep better notes | 16:46 | |
*** pabelanger has left #openstack-oslo | 16:47 | |
*** penguinRaider__ has quit IRC | 16:49 | |
*** flaper87 is now known as flaper87|afk | 16:51 | |
openstackgerrit | OpenStack Proposal Bot proposed a change to openstack/oslo-incubator: Updated from global requirements https://review.openstack.org/111764 | 16:54 |
*** morganfainberg_Z is now known as morganfainberg | 16:55 | |
*** penguinRaider__ has joined #openstack-oslo | 16:57 | |
*** arnaud has quit IRC | 17:00 | |
*** Alexei_987 has quit IRC | 17:19 | |
*** pblaho__ has joined #openstack-oslo | 17:20 | |
ajo | hi YorikSar ;) | 17:23 |
*** ihrachyshka has joined #openstack-oslo | 17:25 | |
*** abhishek has joined #openstack-oslo | 17:34 | |
bnemec | amrith: Make sure you do "with open(...) as f" instead of f = open. I'm pretty sure weird things can happen if the file doesn't get closed/flushed correctly. Using with should take care of that. | 17:40 |
* bnemec has used that method to debug unit tests more than once. | 17:41 | |
amrith | bnemec, ok. | 17:41 |
amrith | will change that. | 17:41 |
*** markmcclain1 has quit IRC | 17:44 | |
*** arnaud has joined #openstack-oslo | 17:49 | |
*** pblaho__ has quit IRC | 17:52 | |
*** praneshp has joined #openstack-oslo | 17:58 | |
*** praneshp has quit IRC | 17:59 | |
*** praneshp has joined #openstack-oslo | 18:00 | |
*** arnaud__ has joined #openstack-oslo | 18:00 | |
*** praneshp has quit IRC | 18:02 | |
*** arnaud has quit IRC | 18:04 | |
*** Alexei_987 has joined #openstack-oslo | 18:06 | |
*** praneshp has joined #openstack-oslo | 18:08 | |
*** ildikov has joined #openstack-oslo | 18:12 | |
*** mriedem has quit IRC | 18:17 | |
*** markmcclain has joined #openstack-oslo | 18:19 | |
*** ihrachyshka has quit IRC | 18:26 | |
openstackgerrit | Andreas Jaeger proposed a change to openstack/oslo-specs: Fix typo https://review.openstack.org/112103 | 18:26 |
openstackgerrit | Andreas Jaeger proposed a change to openstack/oslo-specs: Fix typo, trigger publishing https://review.openstack.org/112103 | 18:28 |
*** arnaud has joined #openstack-oslo | 18:32 | |
*** mriedem has joined #openstack-oslo | 18:36 | |
*** f13o_ has joined #openstack-oslo | 18:45 | |
openstackgerrit | Doug Hellmann proposed a change to openstack/oslo-specs: Add a section to the spec template for testing https://review.openstack.org/112112 | 18:53 |
*** praneshp has quit IRC | 18:55 | |
*** jecarey has quit IRC | 18:58 | |
openstackgerrit | Alessandro Pilotti proposed a change to openstack/oslo-specs: Add a Service Bus for Windows Server RPC backend https://review.openstack.org/109863 | 18:58 |
*** jecarey has joined #openstack-oslo | 19:09 | |
openstackgerrit | Alessandro Pilotti proposed a change to openstack/oslo-specs: Add a Service Bus for Windows Server RPC backend https://review.openstack.org/109863 | 19:21 |
alexpilotti | dhellmann: testing description expanded: https://review.openstack.org/#/c/109863/ | 19:21 |
*** dhellmann is now known as dhellmann_ | 19:22 | |
*** tpatil has joined #openstack-oslo | 19:27 | |
*** alexpilotti has quit IRC | 19:34 | |
*** tpatil_ has joined #openstack-oslo | 19:35 | |
*** tpatil__ has joined #openstack-oslo | 19:36 | |
*** tpatil has quit IRC | 19:37 | |
*** tpatil_ has quit IRC | 19:39 | |
*** tpatil__ has quit IRC | 19:51 | |
amrith | mriedem, got a sec? | 19:51 |
*** stevelle has left #openstack-oslo | 19:52 | |
mriedem | amrith: sure | 19:52 |
amrith | matt, re: your comments on patchset 2. https://review.openstack.org/#/c/111870/2 | 19:53 |
amrith | I pushed a new patchset | 19:53 |
amrith | I'm not sure why the update.py isn't getting the new log.py | 19:53 |
amrith | so, I'm clearly doing something wrong | 19:53 |
amrith | if you have a second, would you be able to help. | 19:54 |
mriedem | amrith: well what's the update.py command that you're running? | 19:55 |
amrith | am just running it again to verify | 19:55 |
amrith | will gist in a second | 19:55 |
mriedem | amrith: if you're targeting processutils w/o log, it's probably not pulling that in since the one commit removed the dep on log.py | 19:55 |
mriedem | so...something like update.py --modules processutils,log (plus other options for where they go) | 19:55 |
mriedem | something like that | 19:55 |
amrith | I asked for module processutils | 19:55 |
mriedem | yeah, which no longer requires log.py | 19:55 |
*** abhishek has quit IRC | 19:55 | |
mriedem | so you have to specificy both | 19:55 |
amrith | I'll try the new command | 19:56 |
amrith | ok | 19:56 |
amrith | mriedem, python update.py --modules processutils,log,strutils,gettextutils --base trove --dest-dir ../trove | 19:58 |
amrith | now grabbed both log.py and jsonutils.py | 19:58 |
amrith | which seems good | 19:59 |
mriedem | amrith: you shouldn't have to specify strutils/gettextutils | 19:59 |
mriedem | just the top level modules | 19:59 |
mriedem | the script will pull in the dependencies | 19:59 |
amrith | you'd said (the last time some weeks ago when we chatted about syncing) that there was a simple way to get the commits. | 19:59 |
mriedem | anyway, tip for the future | 19:59 |
amrith | what would that be? | 19:59 |
amrith | or do you have to troll for them? | 19:59 |
mriedem | amrith: it's not really simple | 19:59 |
mriedem | yes | 19:59 |
mriedem | i check the history of the module in the target project to see when it was last synced | 19:59 |
mriedem | then compare that to the oslo-incubator module's history and try to line them up for what the diff is | 20:00 |
amrith | and look for changes since then | 20:00 |
amrith | ok | 20:00 |
mriedem | then get the shortlog for the oslo modules like: git log --oneline --no-merges openstack/common/processutils.py | 20:00 |
mriedem | something like that | 20:00 |
mriedem | then i list out the changelog for the modules that all got synced | 20:00 |
mriedem | in the commit message | 20:00 |
amrith | ah, neat! | 20:00 |
mriedem | for the diff i mean | 20:00 |
mriedem | rather than just a big list of commits across several modules, then the reviewer has to sort those out, which sucks | 20:01 |
*** dhellmann_ is now known as dhellmann | 20:02 | |
*** f13o_ has quit IRC | 20:03 | |
amrith | mriedem, ok. thx | 20:05 |
mriedem | amrith: good luck | 20:05 |
amrith | yes, I need it on log.py. seems like git log doesn't give a chronological list ;( | 20:06 |
mriedem | git log is chronological | 20:06 |
mriedem | it's the same thing you see on github | 20:06 |
amrith | ok, if that's the case. | 20:08 |
*** tpatil has joined #openstack-oslo | 20:09 | |
amrith | mriedem, would you pl take a look at https://gist.github.com/amrith/101a1e68192932679a36 | 20:10 |
amrith | the dates are not chronological | 20:10 |
amrith | this is in oslo-incubator | 20:10 |
mriedem | amrith: those might be dates that the change was proposed, not merged | 20:11 |
mriedem | or ignores merge commits | 20:11 |
mriedem | amrith: anyway, i wouldn't worry about that too much, the commits are in order, newest to oldest | 20:11 |
amrith | the last time you did a merge of log.py into trove was June 27th | 20:12 |
amrith | so, if I look for commits after June 27th, I should be good. | 20:12 |
amrith | ok, I'll do that. | 20:12 |
*** HenryG has quit IRC | 20:14 | |
amrith | mriedem, thx, just pushed a new patchset. | 20:26 |
openstackgerrit | James Carey proposed a change to openstack/oslo-incubator: Correct coercion of logged message to unicode https://review.openstack.org/110772 | 20:28 |
openstackgerrit | James Carey proposed a change to openstack/oslo-incubator: Add unicode coercion of logged messages to ContextFormatter https://review.openstack.org/112135 | 20:28 |
openstackgerrit | gordon chung proposed a change to openstack/oslo-specs: graduate-oslo-middleware https://review.openstack.org/110353 | 20:41 |
YorikSar | ajo: Hi | 20:42 |
*** openstackgerrit has quit IRC | 21:16 | |
*** openstackgerrit has joined #openstack-oslo | 21:17 | |
*** mriedem has left #openstack-oslo | 21:35 | |
*** tsekiyama has joined #openstack-oslo | 21:35 | |
*** f13o_ has joined #openstack-oslo | 21:37 | |
*** gordc has quit IRC | 21:38 | |
*** jgrimm has joined #openstack-oslo | 21:58 | |
jgrimm | dhellmann, what do you think of idea of going through oslo-incubator and adding list_opts() method through out? a step towards projects using new config-generator when projects are also using incubator code (such as tempest) | 22:02 |
*** jaypipes has quit IRC | 22:05 | |
dhellmann | jgrimm: that makes a lot of sense! | 22:08 |
jgrimm | dhellmann, cool. i couldn't see much of a downside.. just a harmless method | 22:08 |
dhellmann | jgrimm: exactly | 22:09 |
jgrimm | dhellmann, thanks! new generator worked like a champ. easy to understand | 22:09 |
dhellmann | jgrimm: that's what I like to hear | 22:09 |
*** praneshp has joined #openstack-oslo | 22:17 | |
tpatil | Boris-42: Can you please review this patch https://review.openstack.org/#/c/103186/? | 22:21 |
boris-42 | tpatil done | 22:26 |
tpatil | Boris-42: Thank you very much. | 22:27 |
*** jgrimm has quit IRC | 22:55 | |
*** jaosorior has quit IRC | 23:02 | |
*** f13o_ has quit IRC | 23:14 | |
*** markmcclain has quit IRC | 23:48 |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!