| *** kgriffs_afk is now known as kgriffs | 00:04 | |
| *** kgriffs is now known as kgriffs_afk | 00:52 | |
| *** kgriffs_afk is now known as kgriffs | 00:54 | |
| *** malini_afk is now known as malini | 01:16 | |
| *** amitgandhi has quit IRC | 01:16 | |
| *** kgriffs is now known as kgriffs_afk | 01:20 | |
| *** kgriffs_afk is now known as kgriffs | 01:24 | |
| *** kgriffs is now known as kgriffs_afk | 01:31 | |
| *** oz_akan_ has joined #openstack-marconi | 01:45 | |
| *** oz_akan_ has quit IRC | 01:46 | |
| *** oz_akan_ has joined #openstack-marconi | 01:46 | |
| *** kgriffs_afk is now known as kgriffs | 02:26 | |
| *** oz_akan_ has quit IRC | 02:30 | |
| *** oz_akan_ has joined #openstack-marconi | 02:30 | |
| *** oz_akan_ has quit IRC | 02:35 | |
| *** malini is now known as malini_afk | 02:40 | |
| *** kgriffs is now known as kgriffs_afk | 02:46 | |
| *** vkmc has quit IRC | 03:11 | |
| *** oz_akan_ has joined #openstack-marconi | 03:41 | |
| *** oz_akan_ has quit IRC | 03:46 | |
| *** key4 has quit IRC | 06:58 | |
| *** key4 has joined #openstack-marconi | 06:58 | |
| *** flaper87|afk is now known as flaper87 | 07:12 | |
| *** _alexr_ has joined #openstack-marconi | 07:35 | |
| *** ykaplan has joined #openstack-marconi | 09:23 | |
| *** ykaplan has quit IRC | 10:22 | |
| *** fifieldt has quit IRC | 10:29 | |
| *** JRow has joined #openstack-marconi | 11:55 | |
| *** jergerber has joined #openstack-marconi | 12:30 | |
| *** oz_akan_ has joined #openstack-marconi | 12:58 | |
| *** oz_akan_ has quit IRC | 12:59 | |
| *** oz_akan_ has joined #openstack-marconi | 13:00 | |
| *** _alexr_ has quit IRC | 13:25 | |
| *** _alexr_ has joined #openstack-marconi | 13:25 | |
| *** _alexr_ has quit IRC | 13:30 | |
| *** _alexr_ has joined #openstack-marconi | 13:33 | |
| *** kgriffs_afk is now known as kgriffs | 13:33 | |
| *** amitgandhi has joined #openstack-marconi | 13:38 | |
| *** amitgandhi has quit IRC | 13:40 | |
| *** amitgandhi has joined #openstack-marconi | 13:40 | |
| *** malini_afk is now known as malini | 13:51 | |
| kgriffs | malini: can you insert some messages into the test db? | 13:52 |
|---|---|---|
| kgriffs | mar-tst-ord-mng | 13:52 |
| malini | sure | 13:52 |
| kgriffs | thanks! | 13:52 |
| kgriffs | I just need to know the queue name and project ID | 13:53 |
| malini | I'll insert to q18 , 806067 | 13:54 |
| malini | 35K messages is good ? | 13:54 |
| malini | kgriffs: it shud have 36K messages now | 14:00 |
| oz_akan_ | kgriffs: I am working on keystone issue so test environment is not functionally now | 14:00 |
| oz_akan_ | malini: ^^ | 14:00 |
| oz_akan_ | trying to get rid of the middleware to see the performance without it | 14:01 |
| oz_akan_ | I will let you know when it is back | 14:01 |
| malini | oz_akan_ : ok..I got back 201s anyways ;) | 14:02 |
| *** cppcabrera has joined #openstack-marconi | 14:03 | |
| oz_akan_ | malini: please don't run anything, as all queries will go to identity | 14:04 |
| oz_akan_ | there is no caching | 14:04 |
| cppcabrera | Good morning! | 14:04 |
| malini | oz_akan_ : I wont do anything, till you ask me to | 14:04 |
| kgriffs | ok | 14:04 |
| kgriffs | i am just executing queries directly on the master db | 14:04 |
| oz_akan_ | malini: tis, I will let you know | 14:05 |
| flaper87 | malini: morning | 14:05 |
| flaper87 | I commented on your patch | 14:05 |
| malini | flaper87: thanks!!! | 14:05 |
| oz_akan_ | kgriffs: sure that is fine, just we won't be able to insert data through marconi for a while | 14:05 |
| malini | I was just checking that out | 14:05 |
| flaper87 | it'd be really great if you could address those comments soon | 14:05 |
| flaper87 | awesome | 14:05 |
| flaper87 | malini: I already applied my work to your patch | 14:05 |
| malini | flaper87: I am working on tht, right now :) | 14:05 |
| oz_akan_ | cppcabrera: flaper87 good morning and afternoon | 14:05 |
| flaper87 | meaning, I made it depend on yours | 14:05 |
| flaper87 | oz_akan_: cppcabrera kgriffs morning guys | 14:06 |
| malini | flaper87: I am on vacation most of next week & coming back on Friday..I would love to get this merged before I go | 14:06 |
| cppcabrera | I'm going to check out the marconi review queue once I finish sorting through morning email. There's plenty pending there that needs review, last I checked. | 14:06 |
| flaper87 | malini: yes, please | 14:06 |
| cppcabrera | I'll give your patch priority, malini. | 14:06 |
| flaper87 | I promisse to get my things merged before you come back | 14:06 |
| malini | yayy.. | 14:07 |
| flaper87 | as a wb surprise | 14:07 |
| flaper87 | :D | 14:07 |
| cppcabrera | :) | 14:07 |
| flaper87 | so, I split the test package | 14:08 |
| flaper87 | ran tests and it just works | 14:08 |
| cppcabrera | Quick question on a comment for malini's patch, flaper87: why setUp instead of setUpClass for operations that make sense to run once per test suite, rather than once per test case? | 14:09 |
| cppcabrera | +1 flaper87 :D | 14:09 |
| malini | cppcabrera: thanks for asking that :D | 14:09 |
| cppcabrera | Same goes for the tearDown comment. :) | 14:09 |
| malini | like the headers & stuff, we can just do it once | 14:10 |
| flaper87 | cppcabrera: good question, I should've written the motivation of my comment there. Sorry. | 14:11 |
| cppcabrera | No worries, flaper87, so long as we understand the rationale. :) | 14:12 |
| flaper87 | I personally consider the provision of the test environment as something that should run for every test. Meaning, every test should be ensured to have a clean env to work on | 14:12 |
| flaper87 | so, example | 14:12 |
| flaper87 | If test_a writes some messages into the queue created for the whole test suite and fails, test_b will get those messages too | 14:13 |
| flaper87 | and that may break tests in test_b | 14:13 |
| Alex_Gaynor | If anyone has a few minutes, https://review.openstack.org/#/c/44104/ is the first step to getting marconiclient on pypy as well | 14:13 |
| flaper87 | Alex_Gaynor: +1 | 14:14 |
| cppcabrera | +1 Alex_Gaynor | 14:14 |
| Alex_Gaynor | (actually it's the only step, next one after this is adding ito the gate :D) | 14:14 |
| cppcabrera | I see what you're saying flaper87. | 14:14 |
| malini | flaper87: hmm..but tht implies each claim test, should have its own queue ? | 14:15 |
| cppcabrera | Given that the three of us have knowledge of the test operations, I would call the setUpClass an optimization, since expensive operations like calling across the network are performed only once. However, with that optimization in place, if a test were to delete one of the queues created during setUpClass by a new contributor, and we failed to catch that during review, what's the cost? | 14:15 |
| malini | cppcabrera: then it's a bug in the test ;) | 14:16 |
| flaper87 | malini: that implies each claim test will get an empty queue | 14:16 |
| flaper87 | that's part of keeping tests isolated | 14:16 |
| flaper87 | we don't want test_b to fail if test_a fails | 14:16 |
| cppcabrera | PUTing a queue doesn't delete its messages/claims, though, if it already exists. :x | 14:17 |
| cppcabrera | I love the idea of test isolation, though. I agree there. | 14:17 |
| flaper87 | cppcabrera: that's what tearDown should do, right? | 14:17 |
| cppcabrera | Ahh, I see. | 14:17 |
| cppcabrera | I see now. | 14:17 |
| malini | what abt keeping the setupclass for headers etc. , but use setup for each queue creation etc. ? | 14:17 |
| flaper87 | malini: +1 | 14:18 |
| flaper87 | that makes sense to me | 14:18 |
| malini | but, what will happen if we ever have these tests running in parallel ? | 14:18 |
| cppcabrera | flaper87: that's where I advocate using a context manager for creating messages/claims/queues, depending on what's being tested, and setUp/tearDown for consytructing/deleting the primary resource being tested. | 14:18 |
| cppcabrera | malini: ^ | 14:19 |
| flaper87 | malini: in that case we can create a queue per test | 14:19 |
| malini | we'll have multiple tests trying to create the queue etc. - unless we use diff queue names for each test | 14:19 |
| cppcabrera | use the queue name /v1/queues/{uuid.uuid1()} | 14:19 |
| flaper87 | cppcabrera: +1 | 14:19 |
| malini | ok..I'll make those changes | 14:20 |
| flaper87 | malini: awesome! | 14:20 |
| cppcabrera | woot. :) | 14:21 |
| cppcabrera | Then there's the config/base class suggestion | 14:21 |
| cppcabrera | How do you guys feel about putting that off 'til another patch? | 14:21 |
| cppcabrera | malini, flaper87: ^ | 14:21 |
| malini | cppcabrera: why ? | 14:21 |
| flaper87 | mmh, I'd prefer having it in the same patch, it's part of system -> functional refactor | 14:22 |
| flaper87 | and reduces the LOC to review | 14:22 |
| flaper87 | :P | 14:22 |
| cppcabrera | Yeah, good point flaper87. | 14:22 |
| * flaper87 lazy | 14:22 | |
| cppcabrera | I had forgotten the name of the patch, TBH. :P | 14:22 |
| cppcabrera | name/theme | 14:22 |
| malini | flaper87: I dont really understand the whole thing on tht comment..I'll have questions once I get to that | 14:23 |
| flaper87 | malini: sure | 14:24 |
| cppcabrera | Hanging out in #haskell is fun, flaper87. There's lambdas, type signatures, and -> every where. :P | 14:30 |
| flaper87 | cppcabrera: yeaaaah, and haskell's community is pretty nice | 14:31 |
| flaper87 | same happens in #rust | 14:31 |
| cppcabrera | Seems like it. :) | 14:31 |
| cppcabrera | Ooohh, #rust - young and budding. | 14:31 |
| cppcabrera | I haven't been there yet. | 14:31 |
| flaper87 | nice community, nice language as well | 14:32 |
| kgriffs | I've been watching rust, trying to find an excuse to try it. :p | 14:32 |
| flaper87 | cppcabrera: mmh, btw, I should probably have mentioned that #rust is in irc.mozilla.(something) | 14:32 |
| kgriffs | flaper87: remind me why we index first by queue name, not by project? | 14:33 |
| kgriffs | isn't project going to be more selective? | 14:33 |
| cppcabrera | thanks for the directions, flaper87. :) | 14:33 |
| flaper87 | kgriffs: not a real reason. there was a good reason for queue's id | 14:33 |
| kgriffs | what do you mean by a "good reason"? | 14:34 |
| flaper87 | the reason why queue's id was the first in possition is because we could have used it for filtering and nothing else was needed | 14:35 |
| flaper87 | meaning, just using queue_id in the query | 14:35 |
| kgriffs | ok. Let me see if we actually use that anywhere | 14:35 |
| flaper87 | as for queue_name and project, I honestly don't know which one would be better | 14:35 |
| flaper87 | kgriffs: I don't think we're | 14:35 |
| flaper87 | I guess queue_name should go first anyway | 14:36 |
| flaper87 | the probability of having the queue name N times in the index is lower than the probability of having gazillions of queues under the same project | 14:37 |
| zyuan_ | flaper87: i agree | 14:38 |
| zyuan_ | we can leave queue name and project in queues | 14:39 |
| zyuan_ | but not one messages; messages can just join then together | 14:39 |
| cppcabrera | kgriffs: re: the oslo.cache fork you're adding in - does it add anything to the original oslo.cache package, or is it a verbatim copy for the time being? | 14:40 |
| kgriffs | verbatim | 14:40 |
| kgriffs | I will update those patches in a bit; right now I am working on the perf patch | 14:40 |
| cppcabrera | k | 14:41 |
| cppcabrera | I'm just running a round of morning reviews. ;) | 14:41 |
| cppcabrera | I plan on making that part of my routine. | 14:41 |
| flaper87 | cppcabrera: +1 | 14:42 |
| flaper87 | malini: would you be mad if I break your changes in my test split patch? | 14:44 |
| flaper87 | (i'm obviously joking) | 14:44 |
| flaper87 | :D | 14:44 |
| openstackgerrit | Zhihao Yuan proposed a change to stackforge/marconi: fix: claimed message require claim_id to delete https://review.openstack.org/43339 | 14:44 |
| malini | flaper87: yes 8o| | 14:44 |
| malini | seriously ;) | 14:45 |
| zyuan_ | can you review this https://review.openstack.org/#/c/42199/ ? | 14:45 |
| cppcabrera | Alright, all marconi patches have gotten my review. :D | 14:50 |
| cppcabrera | The client side needs a little love. | 14:50 |
| cppcabrera | flaper87: Do you have a spare moment to take a look at reverting common lib (https://review.openstack.org/#/c/42976/) and checking out the message controller (https://review.openstack.org/#/c/37140/)? | 14:51 |
| malini | flaper87, cppcabrera: It does seem to get slower with test level setup | 14:53 |
| flaper87 | cppcabrera: yup, I'll review those | 14:53 |
| flaper87 | malini: I wouldn't be so worried about that, though | 14:53 |
| cppcabrera | malini: It will. Instead of sending http requests once per test suite, it'll do it once per test cases. As flaper87 it saying, though, that's okay. | 14:53 |
| flaper87 | that's the tradeoff we | 14:53 |
| flaper87 | fuck | 14:53 |
| flaper87 | that's the tradeoff we've to pay to keep our tests isolated | 14:54 |
| cppcabrera | s/it/is | 14:54 |
| flaper87 | ah, it is | 14:54 |
| cppcabrera | If and when functional test speed becomes a concern, we can parallelize using the uuid.uuid1() method. | 14:54 |
| cppcabrera | Unit tests, OTOH, should be blazing fast. :D | 14:54 |
| malini | & it'll be faster than now, when running locally with tox.. | 14:55 |
| malini | I am running against a remote server | 14:55 |
| flaper87 | yeah | 14:55 |
| flaper87 | I'm just waiting for your patch | 14:55 |
| flaper87 | :D | 14:55 |
| flaper87 | :D | 14:55 |
| flaper87 | :D | 14:55 |
| cppcabrera | haha | 14:55 |
| openstackgerrit | Zhihao Yuan proposed a change to stackforge/marconi: fix: claimed message require claim_id to delete https://review.openstack.org/43339 | 14:58 |
| malini | have a meeting..will be back in an hour | 15:04 |
| flaper87 | malini: NOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOO | 15:05 |
| flaper87 | :( | 15:05 |
| flaper87 | I already commited my first patch, I just need to rebase and then push for review | 15:05 |
| *** JRow1 has joined #openstack-marconi | 15:06 | |
| *** JRow has quit IRC | 15:08 | |
| *** meganw has joined #openstack-marconi | 15:17 | |
| zyuan_ | cppcabrera: client message controller still depends on apiclient? | 15:22 |
| cppcabrera | lemme double check, zyuan_. Last I checked, I didn't use any of the common lib to implement message controller. | 15:23 |
| cppcabrera | Oops, it does. :P | 15:23 |
| cppcabrera | I borrow one exception class from the common lib. | 15:23 |
| openstackgerrit | Alejandro Cabrera proposed a change to stackforge/python-marconiclient: Implement message controller. https://review.openstack.org/37140 | 15:26 |
| cppcabrera | There - the message controller patch no longer depends on common lib. | 15:27 |
| openstackgerrit | Alejandro Cabrera proposed a change to stackforge/python-marconiclient: Implement message controller. https://review.openstack.org/37140 | 15:37 |
| openstackgerrit | Alejandro Cabrera proposed a change to stackforge/python-marconiclient: Implement message controller. https://review.openstack.org/37140 | 15:51 |
| *** _alexr_ has quit IRC | 15:57 | |
| cppcabrera | brb | 16:06 |
| *** malini is now known as malini_afk | 16:07 | |
| *** malini_afk is now known as malini | 16:08 | |
| flaper87 | cppcabrera: feel free to approve this one | 16:19 |
| flaper87 | malini: are you back? | 16:19 |
| cppcabrera | flaper87: I've yet to figure out how to +2 things. :P | 16:21 |
| cppcabrera | I figured it would come up in the interface, but I see nothing. | 16:21 |
| flaper87 | cppcabrera: just click review and you'll see it | 16:21 |
| flaper87 | ahh wait | 16:21 |
| flaper87 | did you get granted with cow powers? | 16:22 |
| cppcabrera | [-1, 0, +1] | 16:22 |
| cppcabrera | I remember seeing an email about it... | 16:22 |
| * cppcabrera double checks | 16:22 | |
| flaper87 | ahh nevermind, I know what happened | 16:22 |
| flaper87 | :D | 16:22 |
| flaper87 | lp and gerrit are not synced anymore | 16:22 |
| flaper87 | so, this needs to be done in both places | 16:22 |
| cppcabrera | ahh | 16:23 |
| cppcabrera | Yup, I have the lp membership. :P | 16:24 |
| cppcabrera | No gerrit, though, which explains the missing -2, +2 | 16:24 |
| *** flaper87 is now known as flaper87|afk | 16:25 | |
| *** flaper87|afk is now known as flaper87 | 16:26 | |
| flaper87 | cppcabrera: did you get my last messages? | 16:27 |
| flaper87 | cppcabrera: done | 16:27 |
| flaper87 | you should see the +2 and approve thing now | 16:27 |
| cppcabrera | lesseee... | 16:30 |
| cppcabrera | yup, I see the full range now, flaper87. | 16:31 |
| flaper87 | np | 16:31 |
| cppcabrera | thanks. :) | 16:31 |
| flaper87 | cppcabrera: with great power, blah blah blah blah blah | 16:31 |
| flaper87 | :D | 16:31 |
| cppcabrera | haha, of course. :D | 16:31 |
| zyuan_ | :) | 16:31 |
| * cppcabrera approves all the things | 16:31 | |
| flaper87 | LOL | 16:31 |
| zyuan_ | .... | 16:31 |
| cppcabrera | ;) | 16:31 |
| flaper87 | brb, dinner | 16:36 |
| cppcabrera | enjoy! | 16:36 |
| malini | flaper87: back | 16:36 |
| flaper87 | malini: seriously? | 16:37 |
| flaper87 | T_T | 16:37 |
| flaper87 | :P | 16:37 |
| malini | :D | 16:37 |
| malini | will talk after ur dinner :d | 16:37 |
| flaper87 | awesome, thanks | 16:38 |
| flaper87 | how far are you from submitting the new patch? | 16:38 |
| *** whenry has quit IRC | 16:44 | |
| *** flaper87 is now known as flaper87|afk | 16:49 | |
| *** whenry has joined #openstack-marconi | 16:57 | |
| amitgandhi | do we have a list of the possible internal error codes documented anywhere: https://wiki.openstack.org/wiki/Marconi/specs/api/v1#Errors | 17:23 |
| malini | https://wiki.openstack.org/wiki/Marconi/specs/api/v1/errors | 17:23 |
| amitgandhi | im refferring to the internal code here (not the http response codes) | 17:23 |
| amitgandhi | malini: that link gives the http response codes (in the example it says "code": 1092. What is 1092 ? | 17:24 |
| malini | amitgandhi: sorry, I have no clue on tht one | 17:24 |
| amitgandhi | kgriffs: cppcabrera: any idea? | 17:25 |
| kgriffs | internal error codes, like what might be logged? | 17:25 |
| amitgandhi | https://wiki.openstack.org/wiki/Marconi/specs/api/v1#Errors shows a "code": 1092 element | 17:26 |
| amitgandhi | i guess so | 17:26 |
| kgriffs | ah | 17:26 |
| kgriffs | that slipped through the cracks. It isn't implemented | 17:26 |
| kgriffs | let me look for a bp | 17:27 |
| kgriffs | https://blueprints.launchpad.net/marconi/+spec/error-codes | 17:27 |
| kgriffs | if someone is looking for something to do, they can work on it. | 17:27 |
| amitgandhi | lol not urgent | 17:27 |
| amitgandhi | theres more important stuff that needs to be worked on =P | 17:27 |
| kgriffs | kk | 17:28 |
| zyuan_ | kgriffs: can i have a review at https://review.openstack.org/#/c/42199/ ? | 17:36 |
| kgriffs | anybody remember what the new commit message syntax is re bugs? It doesn't look like the wiki has been updated. | 17:42 |
| zyuan_ | kgriffs: Closes-Bug: | 17:42 |
| cppcabrera | kgriffs: https://wiki.openstack.org/wiki/GitCommitMessages | 17:46 |
| cppcabrera | It's as zyuan_ said. | 17:46 |
| cppcabrera | Closes-Bug: #1234567 | 17:46 |
| cppcabrera | Also, Partial-Bug and Related-Bug: | 17:46 |
| kgriffs | oh, then it has been updated | 17:47 |
| cppcabrera | yup. :) | 17:47 |
| kgriffs | thought I remembered it being different than that | 17:47 |
| kgriffs | https://review.openstack.org/#/c/42199/3/marconi/storage/sqlite/utils.py | 17:47 |
| amitgandhi | has anyone looked into the performance of messages._next_marker() | 17:47 |
| kgriffs | zyuan_: ^^^ | 17:47 |
| kgriffs | reviewing now | 17:47 |
| amitgandhi | the find_one appears to be consistently slow looking at newrelic graphs | 17:47 |
| kgriffs | amitgandhi: not yet; remind me to look at that next | 17:48 |
| zyuan_ | kgriffs: ?? | 17:48 |
| amitgandhi | the find() takes about 0.8% of the time, while find_one() is 31% of the time @ 3.8 ms avg | 17:49 |
| kgriffs | noted | 17:49 |
| zyuan_ | kgriffs: it's encode, it can't fail | 17:49 |
| kgriffs | right | 17:50 |
| kgriffs | I am almost done reviewing | 17:50 |
| * amitgandhi wonders if its as simple as order of query (p, q) instead of doing (q, p) to use the correct index? | 17:50 | |
| kgriffs | amitgandhi: order in the query doesn't matter, just when defining the index | 17:52 |
| *** flaper87|afk is now known as flaper87 | 17:56 | |
| flaper87 | malini: ping | 17:56 |
| malini | flaper87: pong | 17:56 |
| flaper87 | malini: news ? | 17:56 |
| malini | I am almost done with the class level setup stuff. | 17:56 |
| malini | I split the queue tests into 3 classes, since setup, teardown is different for insert queue, metadata & all the rest of it | 17:57 |
| cppcabrera | +1 malini | 17:57 |
| malini | but I need help with the config file stuff | 17:58 |
| cppcabrera | When I did that crazy test refactor patch long ago, I did that same class splitting for the queues. :) | 17:58 |
| malini | flaper87: https://review.openstack.org/#/c/43388/4/marconi/tests/functional/README.rst | 17:58 |
| malini | I think we shud continue supporting an extrenal config file, so tht vendors can run tests against their installations | 17:59 |
| malini | as in , I can run the same tests for a Rackspace implementation of marconi | 17:59 |
| flaper87 | malini: we will still support it, my suggestion there is to let oslo.config take care of the path discovery | 18:00 |
| flaper87 | instead of doing that ourselves | 18:00 |
| malini | flaper87: tht sounds cool (except I have no idea how to do that :-$ ) | 18:01 |
| flaper87 | plus, lets use oslo.config's instance instead of having a "get" method for every parameter we need in the test | 18:01 |
| flaper87 | malini: let me help you with that | 18:01 |
| * flaper87 gets some links | 18:01 | |
| malini | flaper87: I will gladly take that :) | 18:01 |
| kgriffs | zyuan: patch approved | 18:02 |
| zyuan_ | thanks! | 18:02 |
| zyuan_ | has anyone used os.fork() in python? | 18:03 |
| flaper87 | zyuan_: o/ | 18:03 |
| zyuan_ | LOL | 18:03 |
| flaper87 | :D | 18:04 |
| kgriffs | can someone sanity-check storage.mongodb.claims:ClaimController.get | 18:04 |
| kgriffs | it looks a little strange to me | 18:04 |
| openstackgerrit | A change was merged to stackforge/marconi: chore: increase coverage in some trivial ways https://review.openstack.org/42199 | 18:04 |
| kgriffs | mostly starting with the try block | 18:04 |
| flaper87 | malini: this call here https://github.com/stackforge/marconi/blob/master/marconi/tests/util/base.py#L61 | 18:04 |
| zyuan_ | kgriffs: what's wrong? | 18:05 |
| flaper87 | malini: will look for the config file under /etc/marconi and ~/.marconi | 18:05 |
| kgriffs | seems like the first message is always eaten | 18:05 |
| kgriffs | also, why claim.pop ? | 18:05 |
| zyuan_ | it's not | 18:05 |
| zyuan_ | wait | 18:05 |
| kgriffs | (nevermind claim variable being overloaded) | 18:05 |
| malini | flaper87: where I do pass in the config file name? | 18:05 |
| kgriffs | if I do next(msgs) | 18:06 |
| kgriffs | that advances the cursor, right? | 18:06 |
| zyuan_ | !!! | 18:06 |
| openstack | zyuan_: Error: "!!" is not a valid command. | 18:06 |
| zyuan_ | \!!! | 18:06 |
| zyuan_ | -_- | 18:06 |
| kgriffs | I don't see that claimed(…) is returning anything special for the first request to the generator | 18:07 |
| flaper87 | malini: that was my next question, why do we need other people to pass the config file for functional tests? I understand about letting then specify what server to talk to | 18:08 |
| flaper87 | kgriffs: mmh, seems you're right | 18:08 |
| flaper87 | it should chain it | 18:08 |
| malini | flaper87: diff implementations can configure marconi differently..like message uplimit yada yada | 18:08 |
| malini | +login details | 18:08 |
| malini | which auth to talk to | 18:09 |
| flaper87 | mmh, but should they run their own marconi-server in that case? | 18:09 |
| flaper87 | and just let the test know where it is | 18:10 |
| flaper87 | ? | 18:10 |
| zyuan_ | flaper87: no problem with it, see line 69 | 18:10 |
| zyuan_ | kgriffs: ^^ | 18:10 |
| flaper87 | ah right | 18:10 |
| zyuan_ | manually chained | 18:10 |
| flaper87 | man, I was like O.O WHAT DID I DOOOO??? | 18:11 |
| flaper87 | :P | 18:11 |
| malini | flaper87: 'should they run their own marconi-server in that case?' —>it wont be a local server, but a remote installation..So (no tox + self running marconi server) in this case | 18:11 |
| flaper87 | mmh, sorry, still don't get it :/ | 18:12 |
| flaper87 | functional tests should be used during marconi's development to test it in live environments, I understand the fact we want those test to be able to talk to a remote marconi | 18:13 |
| malini | flaper87: Lets say Rackspace has a production implementation of Marconi. Its not running on a dev laptop & we need to test it | 18:13 |
| flaper87 | and we can specify that easily | 18:13 |
| flaper87 | malini: sure, totally get that, what I don | 18:13 |
| malini | 'functional tests should be used during marconi's development to test it in live environments' —>this is where we have a gap | 18:13 |
| flaper87 | ah fuck | 18:13 |
| malini | In my veiw, I should be able to run the same tests, if somebody asks me to test in production | 18:14 |
| flaper87 | forget my last, partial message | 18:14 |
| malini | its not just during development phase | 18:14 |
| flaper87 | malini: I guet that, I'm cool with that | 18:14 |
| flaper87 | bad phrasing from my side | 18:14 |
| malini | flaper87: is there a better way to specify vendor configurations, wuthout a functional-tests.conf ? can oslo config handle that? | 18:15 |
| flaper87 | malini: yeah, but I think you need a change I added in my local patch | 18:16 |
| flaper87 | mmhhh | 18:16 |
| flaper87 | lets do this | 18:16 |
| malini | let me just submit what I have now | 18:16 |
| flaper87 | lets let that patch land with the config as is | 18:16 |
| malini | I have the class setups mostly removed (except for one) | 18:17 |
| flaper87 | and I'll refactor the config part when adding the in-test marconi execution | 18:17 |
| malini | sounds good | 18:17 |
| malini | here it comes | 18:17 |
| flaper87 | awesome | 18:17 |
| openstackgerrit | Malini Kamalambal proposed a change to stackforge/marconi: Refactor System Tests https://review.openstack.org/43388 | 18:17 |
| cppcabrera | woot | 18:18 |
| malini | flaper87: aargh..forgot the util part | 18:20 |
| flaper87 | malini: hehe | 18:21 |
| malini | ur suggestion was to move helpers, http & base to functional/ ? | 18:21 |
| flaper87 | I think you can leave it | 18:21 |
| flaper87 | for now | 18:21 |
| flaper87 | I mean, we have util for the other tests as well | 18:22 |
| malini | ok.. | 18:22 |
| flaper87 | I'll remove that in my patch | 18:22 |
| flaper87 | since we're splitting tests | 18:22 |
| malini | thanks :) | 18:22 |
| flaper87 | :) | 18:22 |
| flaper87 | LGTM | 18:26 |
| flaper87 | brb | 18:26 |
| cppcabrera | I'll check it next. | 18:26 |
| flaper87 | cppcabrera: thanks :) | 18:26 |
| malini | yayyyyyy | 18:26 |
| flaper87 | I know there are some flake8 / hacking issues but since it's being ignored for now, I wouldn't boder raising those nits | 18:27 |
| flaper87 | I guess we'll fix them once it runs under tox | 18:27 |
| cppcabrera | +1 flaper87 | 18:27 |
| cppcabrera | +2 - approved; malini. :) | 18:28 |
| flaper87 | I guess it is bother | 18:28 |
| flaper87 | damn, what's wrong w/ me | 18:28 |
| cppcabrera | Too much work, flaper87. ;) | 18:29 |
| flaper87 | cppcabrera: you're right, I know you're right | 18:29 |
| openstackgerrit | A change was merged to stackforge/marconi: Refactor System Tests https://review.openstack.org/43388 | 18:29 |
| cppcabrera | Ithappens to me, too. :) | 18:29 |
| cppcabrera | So I understand, hehe. | 18:29 |
| flaper87 | malini: thanks for the hard work there | 18:29 |
| flaper87 | now, I'll do my part | 18:30 |
| flaper87 | malini: you'll be around tomorrow, right? | 18:30 |
| flaper87 | right? | 18:30 |
| flaper87 | right? | 18:30 |
| flaper87 | right? | 18:30 |
| flaper87 | right? | 18:30 |
| flaper87 | right? | 18:30 |
| flaper87 | :) | 18:30 |
| flaper87 | cppcabrera: yeah, :( | 18:30 |
| flaper87 | I need more brain energy | 18:30 |
| flaper87 | :P | 18:31 |
| malini | flaper87: yes..I'll be here tomorrow | 18:31 |
| malini | we have the hackday, but I would really love to have this whole thing figured out before I leave | 18:31 |
| malini | its merged <:o) !!!!!!! | 18:32 |
| cppcabrera | malini: Yup, merged after 2 +2s. :] | 18:32 |
| flaper87 | malini: cool, I will ask you to review my patches tomorrow before you EOD | 18:33 |
| flaper87 | I want your thoughts about them | 18:33 |
| flaper87 | "them" | 18:33 |
| flaper87 | "them" | 18:33 |
| flaper87 | :D | 18:33 |
| flaper87 | just wanted to emphasize that | 18:33 |
| malini | I will have internet till SUnday morning..SO I can review on saturday too :) | 18:34 |
| flaper87 | malini: I'd never ask you to do that, but I'll make sure you get enough emails from gerrit and let it be the bad guy | 18:34 |
| flaper87 | :D | 18:34 |
| malini | :D | 18:35 |
| flaper87 | kk guys, brb! | 18:35 |
| openstackgerrit | Kurt Griffiths proposed a change to stackforge/marconi: fix: Requests get slower when queues have a lot of messages https://review.openstack.org/44340 | 18:37 |
| cppcabrera | reviewing soon, kgriffs. | 18:40 |
| cppcabrera | Great commit message, btw. | 18:40 |
| kgriffs | flaper87: whats the "pop" for vs. just doing a lookup? | 18:41 |
| kgriffs | 'ttl': claim.pop('t'), | 18:41 |
| * kgriffs is looking | 18:41 | |
| zyuan_ | lookup only is faster | 18:50 |
| zyuan_ | i hope the code does not depends on the side-effect? | 18:51 |
| zyuan_ | flaper87: review~~~ https://review.openstack.org/#/c/43790/ | 18:51 |
| kgriffs | flaper87: http://paste.openstack.org/show/45303/ | 18:56 |
| kgriffs | thoughts? | 18:57 |
| kgriffs | In [49]: %timeit utcnow_ts() | 19:00 |
| kgriffs | 1000000 loops, best of 3: 525 ns per loop | 19:00 |
| kgriffs | In [50]: %timeit utcnow_ts_slow() | 19:00 |
| kgriffs | 100000 loops, best of 3: 3.83 us per loop | 19:00 |
| openstackgerrit | Kurt Griffiths proposed a change to stackforge/marconi: fix: Requests get slower when queues have a lot of messages https://review.openstack.org/44340 | 19:02 |
| openstackgerrit | Alejandro Cabrera proposed a change to stackforge/marconi: feat: marconi proxy https://review.openstack.org/43909 | 19:08 |
| cppcabrera | kgriffs, zyuan: marconi proxy v0.5 is ready for review. I'll be reviewing your mongo opts now, kgriffs. | 19:09 |
| cppcabrera | Maann, I dropped +922 lines on you guys. This is not code review friendly. :P | 19:10 |
| cppcabrera | Sorry about that. | 19:10 |
| cppcabrera | quick question: does it matter if we use BSON dates versus python dates when issuing a query through pymongo? (kgriffs) | 19:19 |
| zyuan_ | we are not using python date; we used integers | 19:19 |
| cppcabrera | zyuan_: timeutils.utcnow() -> datetime.datetime(2013, 8, 29, 19, 21, 44, 929690) | 19:22 |
| cppcabrera | query = {..., 'e': {'$lte': timeutils.utcnow()}, ...} | 19:22 |
| cppcabrera | so we are using python dates in mongodb.messages:_removed_expired | 19:23 |
| zyuan_ | cppcabrera: ... | 19:24 |
| cppcabrera | "Note that documents can contain native Python types (like datetime.datetime instances) which will be automatically converted to and from the appropriate BSON types." | 19:24 |
| kgriffs | seems like we could just use timestamps there | 19:24 |
| cppcabrera | Cool, it's not a problem. | 19:24 |
| kgriffs | save a little room, maybe faster query | 19:24 |
| cppcabrera | That's from the pymongo docs ^^ | 19:24 |
| cppcabrera | we'd save a bit of space, I'm sure. Sounds like one of those potential optimizations. | 19:25 |
| zyuan_ | it does not save space | 19:30 |
| zyuan_ | http://bsonspec.org/#/specification | 19:30 |
| *** JRow1 has quit IRC | 19:30 | |
| zyuan_ | where bson datetime type is just a int64 | 19:30 |
| zyuan_ | milisecods from epoch | 19:30 |
| cppcabrera | Ahh, good find zyuan_ | 19:30 |
| kgriffs | https://gist.github.com/kgriffs/6382507 | 19:41 |
| kgriffs | who knows, it may add up over time? :p | 19:41 |
| cppcabrera | phew, 8x more nanoseconds | 19:41 |
| cppcabrera | :) | 19:42 |
| zyuan_ | that's for sure... | 19:42 |
| kgriffs | also consider the conversion from datetime to bson timestamp | 19:42 |
| kgriffs | may add another 200 ns | 19:42 |
| * kgriffs runs for the hills | 19:42 | |
| cppcabrera | lol | 19:42 |
| zyuan_ | double vs. struct | 19:42 |
| cppcabrera | There's a catch, though. | 19:42 |
| zyuan_ | what i did in sqlite is to store double | 19:43 |
| cppcabrera | is time.time() UTC-based? | 19:43 |
| kgriffs | srsly though, if you optimize a few of these and shave a few microseconds, it can be significant in terms of throughput | 19:43 |
| kgriffs | cppcabrera: unix timestamp | 19:43 |
| cppcabrera | I need to refresh on UNIX timestamp's relationship with timezones. >.> | 19:43 |
| cppcabrera | ahh, UTC | 19:44 |
| cppcabrera | cool | 19:44 |
| kgriffs | heh | 19:44 |
| zyuan_ | i used julianday | 19:44 |
| kgriffs | it's just number of seconds since an arbitrary epoch | 19:44 |
| cppcabrera | alright, time.time() is the way to go in the future for another improvement | 19:44 |
| kgriffs | added to the work items | 19:46 |
| kgriffs | https://blueprints.launchpad.net/marconi/+spec/v1-obvious-optimizations | 19:46 |
| kgriffs | note that if we use timeutils.utcnow_ts() we should submit a patch to use time.time() as well in that function | 19:46 |
| kgriffs | (see my earlier paste) | 19:47 |
| * kgriffs thinks it is lame to slow things down for the sake of testability when there's any easy alternative | 19:47 | |
| kgriffs | see also: http://paste.openstack.org/show/45303/ | 19:47 |
| openstackgerrit | Kurt Griffiths proposed a change to stackforge/marconi: chore: Update openstack.common, add lockutils https://review.openstack.org/44105 | 19:49 |
| cppcabrera | here's the full impact of time.time() vs. datetime.datetime.now() running against mongo: http://paste.openstack.org/show/45419/ | 19:50 |
| cppcabrera | kgriffs ^^ | 19:50 |
| kgriffs | that's ms, right? | 19:50 |
| cppcabrera | That's seconds. | 19:51 |
| cppcabrera | 7.18 seconds vs 6.75 seconds | 19:51 |
| kgriffs | oh, I see, you didn't divide by number | 19:51 |
| cppcabrera | I drop()'d the collection between runs to make the amount of data in the DB didn't affect the test. | 19:51 |
| kgriffs | looks like ~21 microseconds | 19:52 |
| kgriffs | that's actually significant | 19:52 |
| kgriffs | (difference, per attempt on average) | 19:53 |
| * cppcabrera loves timeit for microbenchmarks | 19:53 | |
| kgriffs | btw, closures work too | 19:54 |
| kgriffs | timeit.repeat(my_func) | 19:54 |
| kgriffs | min(timeit.repeat(my_func)) / number | 19:55 |
| cppcabrera | hmmm... I had no idea functions could be passed to timeit. | 19:57 |
| kgriffs | cppcabrera: this should be good to go now: https://review.openstack.org/#/c/44105/1 | 20:01 |
| cppcabrera | I just +2'd it before coming back to IRC, heh. | 20:02 |
| cppcabrera | brb | 20:03 |
| flaper87 | back | 20:04 |
| cppcabrera | me, too. | 20:05 |
| cppcabrera | wb, flaper87. :) | 20:05 |
| flaper87 | cppcabrera: you too :D | 20:05 |
| *** malini is now known as malini_afk | 20:05 | |
| kgriffs | flaper87: https://review.openstack.org/#/c/44105/1 | 20:06 |
| cppcabrera | The hot, pending patches for the day are: https://review.openstack.org/#/c/44105/ (lock utils into marconi), marconi proxy (https://review.openstack.org/#/c/43909/), fix slowdown as messages are added (https://review.openstack.org/#/c/44340/), add oslo.cache to marconi (https://review.openstack.org/#/c/44131/) (flaper87, kgriffs) | 20:07 |
| flaper87 | kgriffs: +2 | 20:07 |
| openstackgerrit | A change was merged to stackforge/marconi: feat(wsgi): homedoc now ships relative URIs https://review.openstack.org/43790 | 20:07 |
| kgriffs | w00t | 20:07 |
| cppcabrera | sweet - 3 patches to go. marconi-proxy is a beast of a patch. :P | 20:07 |
| flaper87 | wow | 20:08 |
| flaper87 | cppcabrera: mind If I review the proxy patch tomorrow? | 20:08 |
| cppcabrera | that's cool, flaper87. :) | 20:08 |
| flaper87 | I don't think I've the right mind power to review it as it should | 20:08 |
| cppcabrera | I'll be adding unit tests to it later today. I barely have the mind to review my own patch atm. | 20:08 |
| flaper87 | cppcabrera: the only thought I've right now is: Can it be split in 2 or more patches? | 20:08 |
| flaper87 | if so, it would be really nice to do it | 20:09 |
| openstackgerrit | Kurt Griffiths proposed a change to stackforge/marconi: fix: Requests get slower when queues have a lot of messages https://review.openstack.org/44340 | 20:09 |
| openstackgerrit | A change was merged to stackforge/marconi: chore: Update openstack.common, add lockutils https://review.openstack.org/44105 | 20:09 |
| cppcabrera | flaper87: I'll try to split it. I think I can make it so that the first patch adds the core catalogue stuff, the second patch forwards things to marconi, and maybe another split. | 20:10 |
| flaper87 | cppcabrera: awesome, that would be awesome | 20:11 |
| cppcabrera | I've read the studies on code review, too, and I know there's fierce diminishing returns after about 350~ LOC. :P | 20:11 |
| cppcabrera | I remember seeing it as a hot topic on openstack-dev@ (code review studies) | 20:12 |
| kgriffs | this one also needs some love | 20:12 |
| kgriffs | https://review.openstack.org/#/c/43339/ | 20:12 |
| cppcabrera | Ahh, yes. That one affects our API's behavior. | 20:13 |
| cppcabrera | Makes it a little stricter about claims handling. | 20:13 |
| kgriffs | is_claimed = 'c' in message and message['c']['e'] > now | 20:13 |
| kgriffs | c is always in message now, or am I wrong? | 20:14 |
| kgriffs | yes, we need it to be feature complete (forgot it wasn't in yet) | 20:14 |
| flaper87 | kgriffs: c is always present | 20:14 |
| kgriffs | zyuan_: ^^^ | 20:14 |
| flaper87 | (assuming we're talking about mongodb's driver) | 20:14 |
| kgriffs | https://review.openstack.org/#/c/43339/3/marconi/storage/mongodb/messages.py | 20:14 |
| kgriffs | yes | 20:14 |
| kgriffs | I'm reviewing it now | 20:14 |
| zyuan_ | let'me check | 20:16 |
| zyuan_ | yea | 20:17 |
| zyuan_ | 'c' always exists; c.id may be None | 20:17 |
| kgriffs | ok, second thing | 20:18 |
| kgriffs | messagenotpermitted and claimnotpermitted don't seem to be appropriate names for the errors | 20:18 |
| kgriffs | how about something more like "DeleteNotPermitted" | 20:18 |
| kgriffs | (explicit) | 20:19 |
| zyuan_ | there base is NotPermitted | 20:19 |
| zyuan_ | ...out api only has 1 "not permitted" concept | 20:19 |
| kgriffs | yes, I know, but that doesn't prevent us from renaming the child classes | 20:19 |
| zyuan_ | ClaimDeletionNotPermitted? | 20:20 |
| kgriffs | you aren't deleting a claim... | 20:20 |
| zyuan_ | (don't remind me java -_-) | 20:20 |
| zyuan_ | oops | 20:20 |
| kgriffs | you are deleting a message | 20:20 |
| kgriffs | and it isn't permitted, right? | 20:21 |
| zyuan_ | NotPermittedByClaim | 20:21 |
| kgriffs | do you need two different error types for the two cases? | 20:21 |
| zyuan_ | ... | 20:21 |
| zyuan_ | for wsgi, no | 20:21 |
| zyuan_ | but it's good for storage | 20:21 |
| zyuan_ | just like we have different types of NotExisting | 20:22 |
| kgriffs | I mean, could you just raise something like DeleteNotPermitted if the message is claimed, but either the caller did not specify a claim ID, or they did but it was invalid? | 20:22 |
| openstackgerrit | Kurt Griffiths proposed a change to stackforge/marconi: chore: Add the up-and-coming oslo.cache module https://review.openstack.org/44131 | 20:24 |
| zyuan_ | kgriffs: that's currently what wsgi understand, but i don't think storage should do this | 20:25 |
| zyuan_ | those errors are not echo to users anyway | 20:25 |
| openstackgerrit | Zhihao Yuan proposed a change to stackforge/marconi: fix: claimed message require claim_id to delete https://review.openstack.org/43339 | 20:26 |
| zyuan_ | someone broke the unittests' config file i guess | 20:26 |
| amitgandhi | kgriffs: the task to look into find_one() performance − should that be a bp/bug/or added as a workitem to https://blueprints.launchpad.net/marconi/+spec/v1-obvious-optimizations | 20:28 |
| kgriffs | bug | 20:28 |
| zyuan_ | bug +1 | 20:28 |
| kgriffs | zyuan_: if you want to raise different types, that's fine, but the current names imply that "a message is not permitted" and that a "claim is not permitted" | 20:29 |
| kgriffs | (doesn't make much sense in this context) | 20:29 |
| zyuan_ | try rename it... | 20:30 |
| zyuan_ | but... v_v malini_afk -_- fix nosetests | 20:30 |
| openstackgerrit | Zhihao Yuan proposed a change to stackforge/marconi: fix: claimed message require claim_id to delete https://review.openstack.org/43339 | 20:31 |
| zyuan_ | WIP | 20:31 |
| zyuan_ | MessageIsNotClaimed | 20:33 |
| zyuan_ | and | 20:33 |
| zyuan_ | MessageIsNotClaimedBy ? | 20:33 |
| amitgandhi | perf bug reported: https://bugs.launchpad.net/marconi/+bug/1218602 | 20:33 |
| zyuan_ | thanks | 20:34 |
| openstackgerrit | Zhihao Yuan proposed a change to stackforge/marconi: fix: claimed message require claim_id to delete https://review.openstack.org/43339 | 20:36 |
| zyuan_ | ok | 20:36 |
| kgriffs | amitgandhi: thanks! | 20:37 |
| kgriffs | can someone pls confirm https://bugs.launchpad.net/marconi/+bug/1218602 | 20:37 |
| flaper87 | kgriffs: I was looking at it. I guess we could try .find().limit(1) + index hint | 20:38 |
| flaper87 | or keeping the marker somewhere else | 20:38 |
| amitgandhi | i noticed that we hit q and then p | 20:38 |
| amitgandhi | does that matter | 20:38 |
| kgriffs | nope | 20:38 |
| zyuan_ | i guess not | 20:38 |
| flaper87 | no | 20:39 |
| amitgandhi | ie does it screw up the way the index is used | 20:39 |
| zyuan_ | the last entry is hard to reach | 20:39 |
| zyuan_ | there is just no index on marker | 20:39 |
| amitgandhi | you would want to filter p and then by q to reduce the scan (idk if mongo is smart enough to rearrange the filter clauses like sql is) | 20:40 |
| flaper87 | amitgandhi: yeah, there's a patch for that | 20:40 |
| flaper87 | kurt added it | 20:40 |
| amitgandhi | oic | 20:40 |
| flaper87 | s/added/submitted/ | 20:40 |
| flaper87 | amitgandhi: good thought, anyway | 20:40 |
| kgriffs | amitgandhi: i'm certain it doesn't matter, otherwise pymongo would require a list of tuples | 20:41 |
| flaper87 | kgriffs: I think he means in the index | 20:42 |
| flaper87 | not in the query | 20:42 |
| kgriffs | I went ahead an reorded in my patch anyway just to be consistent with the index definitions | 20:42 |
| flaper87 | I guess | 20:42 |
| kgriffs | oic | 20:42 |
| flaper87 | well, it makes sense in the index | 20:42 |
| flaper87 | kgriffs: and I +1 you for that | 20:42 |
| flaper87 | there are more messages in a queue than queues in a project | 20:43 |
| flaper87 | well, that's what my random probability calculation says | 20:43 |
| flaper87 | kgriffs: btw, what do you think about merging p and q in the same field | 20:43 |
| flaper87 | ? | 20:43 |
| flaper87 | in the message collection | 20:43 |
| flaper87 | that should reduce the number of indexes and indexes lookup | 20:43 |
| flaper87 | I mean, the size of the index | 20:44 |
| kgriffs | intriguing | 20:44 |
| flaper87 | and the fields to filter on | 20:44 |
| flaper87 | damn, today is not my chatting day | 20:44 |
| flaper87 | and that should also make the query faster | 20:44 |
| * flaper87 has a background thread thinking on further optimizations | 20:45 | |
| kgriffs | I thought of that a long time ago, but forgot about it since we were originally querying on the queue ID | 20:45 |
| flaper87 | kgriffs: yeah, that didn't make sense before | 20:45 |
| flaper87 | it does now though | 20:45 |
| zyuan_ | i think it's easier to store an sha256 for q+'+'+p | 20:45 |
| kgriffs | I don't see any reason not to try it unless for some reason we only want to query one of those in isolation | 20:46 |
| kgriffs | I suppose we could use a regex for that (don't tell anyone) | 20:46 |
| flaper87 | kgriffs: EXACTLY | 20:46 |
| flaper87 | :D | 20:46 |
| kgriffs | if it's a prefix pattern, then it can still use the index | 20:46 |
| flaper87 | exactly | 20:46 |
| flaper87 | :D | 20:46 |
| kgriffs | so... | 20:46 |
| zyuan_ | al message need is unique constrain by queue and project | 20:46 |
| kgriffs | if you need all things in a project | 20:46 |
| zyuan_ | need to go the /queues endpoint | 20:47 |
| kgriffs | ^project_id-.* | 20:47 |
| flaper87 | kgriffs: yup | 20:47 |
| kgriffs | anyway... | 20:47 |
| kgriffs | feel free to give that a try | 20:47 |
| flaper87 | sure, I will | 20:47 |
| flaper87 | lets get yours merged first | 20:47 |
| kgriffs | sure | 20:47 |
| zyuan_ | flaper87: i recommand do this: | 20:49 |
| zyuan_ | a=hashlib.sha256("project") | 20:49 |
| zyuan_ | a.update("queue") | 20:50 |
| kgriffs | sha256 isn't exactly fast | 20:50 |
| zyuan_ | a.digest() # bytes | 20:50 |
| flaper87 | yeah, I'd prefer keeping it plain | 20:50 |
| zyuan_ | but it's always 32bytes | 20:50 |
| kgriffs | true, it is a space-time tradeoff | 20:50 |
| flaper87 | mmh | 20:50 |
| zyuan_ | short=faster to __hash__ | 20:51 |
| kgriffs | although if you start having to page then smaller is faster too | 20:51 |
| kgriffs | maybe one step at a time | 20:51 |
| kgriffs | merging them as plaintext will already save a little RAM | 20:52 |
| zyuan_ | just use update() can save an intermediate string | 20:52 |
| flaper87 | the q+p will end up in a utility function | 20:52 |
| kgriffs | then you have a baseline for performance testing if you want to hash | 20:52 |
| flaper87 | which will be easier to update in the future | 20:52 |
| zyuan_ | no string contenation is needed | 20:52 |
| flaper87 | with some hashing | 20:52 |
| zyuan_ | q+p needs allocation | 20:52 |
| kgriffs | yes, but having a single index field vs two should save some overhead | 20:52 |
| zyuan_ | while update is just a scan on source | 20:52 |
| kgriffs | also a little bson overhead | 20:53 |
| flaper87 | zyuan_: I know, I just haven't figure out a name for the function | 20:53 |
| flaper87 | ^^ | 20:53 |
| flaper87 | figured | 20:53 |
| zyuan_ | hmmm, wait. considering the use case, it may not be short. ever seen project + queue longer than 32 chars? | 20:54 |
| zyuan_ | V_V | 20:54 |
| kgriffs | I imagine we will see a lot in the 40-60 range | 20:55 |
| kgriffs | queue names may be uuids | 20:55 |
| kgriffs | (36 chars just for that) | 20:55 |
| zyuan_ | well | 20:55 |
| openstackgerrit | Alejandro Cabrera proposed a change to stackforge/marconi: feat: marconi proxy https://review.openstack.org/43909 | 20:57 |
| cppcabrera | patch split part 1: the proxy-specific operations - no forwarding ^^ | 20:57 |
| zyuan_ | interesting. sha512 is faster than sha256 | 20:57 |
| *** meganw has quit IRC | 20:57 | |
| zyuan_ | but 2x longer result | 20:57 |
| flaper87 | cppcabrera: +1 thanks for that | 20:57 |
| cppcabrera | ~644 LOC now. :P | 20:58 |
| cppcabrera | kgriffs: If I do 'pip install falcon' atm, does it include the set_default_route patch? | 20:58 |
| flaper87 | kgriffs: btw, we've to answer the question w.r.t "Why falcon and not Pecan" in the next meeting. | 20:59 |
| flaper87 | kgriffs: you had some benchmarks about that, right? (different from the ones on the website) | 20:59 |
| zyuan_ | cppcabrera: marconi's version restriction .... | 21:00 |
| cppcabrera | zyuan_: that's true, also, falcon's new patch hasn't been pushed to pypi just yet. I verified. :( | 21:00 |
| kgriffs | cppcabrera: no | 21:00 |
| kgriffs | cppcabrera: will be in 0.1.7 | 21:01 |
| kgriffs | if anyone wants to help finish that release during a hack day, be my guest! | 21:02 |
| kgriffs | flaper87: there is a complex benchmark that you can run as a command after installing falcon... | 21:03 |
| kgriffs | but | 21:03 |
| kgriffs | it doesn't do the same complex things in all the other frameworks, only benchmarks falcon | 21:03 |
| kgriffs | you can run the simple benchmark | 21:03 |
| kgriffs | then run the complex one | 21:03 |
| kgriffs | and if the complex one is still a lot faster than the simple one for the other frameworks, you win. :D | 21:04 |
| kgriffs | which is the case last time I checked | 21:04 |
| flaper87 | mmh, ok! | 21:05 |
| kgriffs | the real question is what would be the perf difference with a real app | 21:05 |
| flaper87 | Do we have other reasons / answers for that question? | 21:05 |
| flaper87 | kgriffs: yeah | 21:05 |
| kgriffs | so we would really have to spent a fair amoutn of time writing a pecan driver to find out | 21:05 |
| flaper87 | my question actually is, Why does it matter so much? | 21:06 |
| kgriffs | it is based on past experience with a proprietary Rackspace infrastructure service | 21:06 |
| flaper87 | what's the big issue about having falcon as a library in the global requirements | 21:06 |
| flaper87 | ? | 21:06 |
| kgriffs | that framework speed matters in the data plane | 21:06 |
| flaper87 | kgriffs: no no, I wasn't talking about the speed | 21:07 |
| kgriffs | you mean, why does anyone care that we take a dep on falcon? | 21:07 |
| flaper87 | kgriffs: yup | 21:07 |
| kgriffs | Doug is on a crusade to standardize web frameworks across openstack projects | 21:07 |
| kgriffs | (and I mean "crusade" in the nicest of ways) | 21:07 |
| flaper87 | yeah, I've helped a bit on that, TBH | 21:08 |
| kgriffs | I am not opposed to the spirit of the idea | 21:08 |
| kgriffs | BUT | 21:08 |
| flaper87 | but, mmh, well, I guess that makes contributors life easier | 21:08 |
| flaper87 | as well | 21:08 |
| kgriffs | I'm not sure that Pecan was the best choice if he was hoping to get marconi and swift on board | 21:08 |
| kgriffs | I'm not saying this because I wrote falcon | 21:09 |
| kgriffs | I didn't want to write falcon | 21:09 |
| kgriffs | but it was necessary | 21:09 |
| flaper87 | I know, we're just talking about what the best thing for projects is | 21:09 |
| flaper87 | or guessing, at least | 21:09 |
| kgriffs | now, all this being said, I think we really should try to take some time and write a pecan driver at some point | 21:09 |
| flaper87 | :D | 21:09 |
| *** oz_akan_ has quit IRC | 21:10 | |
| flaper87 | kgriffs: kk | 21:10 |
| openstackgerrit | Alejandro Cabrera proposed a change to stackforge/marconi: feat: marconi proxy (v1, health) https://review.openstack.org/44356 | 21:10 |
| flaper87 | I guess we can propose that as part of the incubation process | 21:10 |
| kgriffs | swift might try it as well. | 21:10 |
| flaper87 | I mean, we can write that during incubation | 21:10 |
| kgriffs | but I don't want to put them in the middle of this if they don't want to be | 21:10 |
| flaper87 | and decide before being integrated | 21:10 |
| kgriffs | correct | 21:10 |
| torgomatic | pecan seems to have blocking IO in the web framework --> one thread or process per connection --> something like a 0.00% chance of ever landing in Swift | 21:10 |
| flaper87 | awesome | 21:10 |
| * notmyname isn't familiar with either pecan or falcon, other than knowing people seem to think they should be used everywhere | 21:11 | |
| notmyname | torgomatic: ah. well then | 21:11 |
| kgriffs | hah | 21:11 |
| flaper87 | torgomatic: owww really???? | 21:11 |
| torgomatic | notmyname: this is from a cursory glance, though, not a deep dive | 21:11 |
| flaper87 | mmhh, interesting | 21:11 |
| torgomatic | maybe it can be used together with eventlet or gevent, in which case it'd be usable | 21:11 |
| torgomatic | in Swift, that is | 21:11 |
| torgomatic | but as it is, it seems like there's no provision for handling many client connections in a single Python process | 21:12 |
| * torgomatic is happy to be proven wrong, though | 21:12 | |
| kgriffs | my perspective is that it's a web app framework | 21:13 |
| flaper87 | I guess we should ask markmacclain | 21:13 |
| kgriffs | the creators didn't envision it for large-scale web services | 21:13 |
| kgriffs | but I digress | 21:13 |
| kgriffs | FWIW, I haven't been pushing falcon in the community. Other people have suggested using it in other projects, but I have been trying to steer the discussions in a more collaborative vs. adversarial direction. | 21:14 |
| kgriffs | I'm sure that since OpenStack started using pecan (introduced by Doug, who works at dreamhost, where Pecan was born) | 21:15 |
| kgriffs | there has been some effort to make it better for web services | 21:15 |
| kgriffs | anyway, I'm speculating so don't quote me on any of this. :p | 21:16 |
| kgriffs | flaper87: one more thing | 21:16 |
| kgriffs | it's hard to make money on a queuing service. So operators will need it to be as efficient as possible. | 21:17 |
| kgriffs | if a pecan driver is elegant and really fast, then sure, why not? | 21:17 |
| kgriffs | but if not, I think pragmatism has to have it's due | 21:18 |
| * kgriffs isn't at all opinionated | 21:19 | |
| * ametts loves it when kgriffs says "Don't quote me on this" in a public IRC channel :) | 21:20 | |
| kgriffs | heh | 21:20 |
| kgriffs | if only you knew what I was *really* thinking but didn't say! | 21:20 |
| kgriffs | <jk | 21:20 |
| flaper87 | kgriffs: yeah, I completely agree with you. That's why I see it as a "Ok, lets try this thing first and we'll see" | 21:21 |
| flaper87 | as part of the incubation period | 21:21 |
| kgriffs | yep, I think that is our answer | 21:21 |
| flaper87 | I'm not opinionated but I do wan't what's best for the project | 21:21 |
| kgriffs | +1 | 21:21 |
| flaper87 | and I've been appliying the same way of thinking to every other project throughout OpenStack | 21:21 |
| flaper87 | so, I expect others to do the same w/ Marconi | 21:22 |
| kgriffs | it isn't "us vs. them", it's just "us" | 21:22 |
| flaper87 | exactly | 21:22 |
| flaper87 | TBH, I don't think it'll be an issue, at all | 21:22 |
| flaper87 | as long as we're open to try Pecan and pick the one that fits better for marconi | 21:23 |
| kgriffs | sure, it only makes sense | 21:24 |
| openstackgerrit | Alejandro Cabrera proposed a change to stackforge/marconi: feat: marconi proxy https://review.openstack.org/43909 | 21:24 |
| openstackgerrit | Alejandro Cabrera proposed a change to stackforge/marconi: feat: marconi proxy (v1, health) https://review.openstack.org/44356 | 21:25 |
| openstackgerrit | Kurt Griffiths proposed a change to stackforge/marconi: chore: Track the up-and-coming oslo.cache module https://review.openstack.org/44131 | 21:31 |
| openstackgerrit | Alejandro Cabrera proposed a change to stackforge/marconi: feat: marconi-proxy forwarding https://review.openstack.org/44360 | 21:31 |
| kgriffs | folks: ^^^ | 21:31 |
| amitgandhi | so many propositions | 21:31 |
| kgriffs | I added a test to make sure namespaces were correct, and found some bugs. :p | 21:31 |
| kgriffs | should be good to go now | 21:31 |
| flaper87 | kgriffs: in the cache lib? | 21:31 |
| kgriffs | yes | 21:32 |
| kgriffs | since I moved it under marconi.common | 21:32 |
| flaper87 | kgriffs: something I should port to the oslo patch? | 21:32 |
| flaper87 | ah ok | 21:32 |
| kgriffs | no, ur patch is fine | 21:32 |
| flaper87 | kk | 21:32 |
| flaper87 | kgriffs: btw, it got some extra reviews, in case you want to comment. I'll do that tomorrow | 21:33 |
| kgriffs | ok | 21:34 |
| kgriffs | on that note | 21:34 |
| kgriffs | what do you think about a decorator that will call _prepare_key for you? | 21:35 |
| kgriffs | http://paste.openstack.org/show/45425/ | 21:35 |
| kgriffs | (side-by-side comparison of how it might look) | 21:35 |
| flaper87 | There are 2 reasons why I'd prefer not having it: The first one is that it assumes there's a key to transform in the first argument, which shouldn't be a big deal since the API defines the methods that way. The second is that there are cases where having the original key is needed https://review.openstack.org/#/c/42878/2/openstack/common/cache/_backends/memcached.py | 21:38 |
| flaper87 | get_many | 21:38 |
| flaper87 | and third, that it would make sense just for set, unset | 21:38 |
| kgriffs | why not get | 21:39 |
| flaper87 | while the *_many methods will have to call it manually | 21:39 |
| flaper87 | kgriffs: and get, sorry | 21:39 |
| flaper87 | :D | 21:39 |
| flaper87 | those are just some thoughts, I'm not compeltely against the decorator | 21:39 |
| kgriffs | incr, add | 21:40 |
| flaper87 | I definitely hate calling that method everytime | 21:40 |
| kgriffs | looks like the _many don't fit nicely in that model, I agree | 21:40 |
| kgriffs | hmm, well, something to noodle on | 21:40 |
| kgriffs | if you can figure out a nice way to avoid having to call that every time, that would be AWESOME | 21:41 |
| kgriffs | aaaaanyway | 21:41 |
| kgriffs | I gotta run | 21:41 |
| flaper87 | kgriffs: couldn't agree more! | 21:41 |
| flaper87 | kgriffs: take care :) | 21:41 |
| flaper87 | ttyl | 21:41 |
| kgriffs | flaper87: pls review the cache patch for marconi | 21:41 |
| kgriffs | https://review.openstack.org/#/c/44131/ | 21:42 |
| flaper87 | kgriffs: yes, I will | 21:42 |
| kgriffs | thanks! | 21:42 |
| kgriffs | have a good one, | 21:42 |
| kgriffs | u around tomorrow? | 21:42 |
| flaper87 | kgriffs: yes, I'll be here | 21:43 |
| flaper87 | I'm planning to close the tests patches tomorrow | 21:43 |
| flaper87 | and help you with the performance ones | 21:43 |
| kgriffs | excellent | 21:43 |
| kgriffs | btw | 21:43 |
| flaper87 | shoot | 21:43 |
| flaper87 | :) | 21:43 |
| kgriffs | https://blueprints.launchpad.net/marconi/+spec/v1-obvious-optimizations | 21:43 |
| kgriffs | I added some work items to the bottom (just brainstorming) | 21:43 |
| flaper87 | ah, nice. Awesome. That's helpful | 21:44 |
| flaper87 | thanks | 21:44 |
| kgriffs | this too | 21:44 |
| kgriffs | http://paste.openstack.org/show/45303/ | 21:44 |
| kgriffs | its like 7x faster | 21:44 |
| kgriffs | takes it down to nanosecond range | 21:44 |
| flaper87 | why is calendar being used in first place? | 21:45 |
| flaper87 | I mean, in oslo | 21:45 |
| flaper87 | mmh | 21:45 |
| kgriffs | to reuse utcnow | 21:45 |
| flaper87 | ah right | 21:45 |
| kgriffs | afaict | 21:45 |
| flaper87 | mmh | 21:45 |
| flaper87 | kk, sounds like something we can contribute back to oslo | 21:45 |
| kgriffs | definitely | 21:45 |
| flaper87 | kgriffs: +! | 21:45 |
| flaper87 | kgriffs: +1 | 21:45 |
| kgriffs | would we need to open a bug first? | 21:45 |
| kgriffs | or just submit the patch? | 21:46 |
| flaper87 | I'd say just submit a patch with a good description | 21:46 |
| kgriffs | ok | 21:46 |
| kgriffs | will do | 21:46 |
| flaper87 | awesome | 21:46 |
| kgriffs | someone may have a better idea of how to "fix" this, but this was the first approach that came to mind | 21:46 |
| *** flaper87 is now known as flaper87|afk | 21:51 | |
| openstackgerrit | Alejandro Cabrera proposed a change to stackforge/marconi: feat: marconi-proxy forwarding https://review.openstack.org/44364 | 21:56 |
| cppcabrera | Finally got those dependencies right. :P | 21:56 |
| cppcabrera | Three patches: catalogue, v1 + health, full-on forwarding | 21:56 |
| cppcabrera | Still needs unit tests. | 21:56 |
| kgriffs | https://review.openstack.org/#/c/44363/ | 21:57 |
| cppcabrera | But that's the core with added docstrings @ [644, 70, 160] LOC | 21:57 |
| kgriffs | timeutils patch ^^^ | 21:57 |
| kgriffs | cppcabrera: cool, thanks | 21:57 |
| kgriffs | I gotta run | 21:57 |
| kgriffs | I'll have to check it out tomorrow | 21:57 |
| kgriffs | thanks everyone for you awesome work | 21:58 |
| kgriffs | ttfn | 21:58 |
| cppcabrera | kgriffs: o/ | 21:58 |
| cppcabrera | I'm out for the night, too. | 21:58 |
| cppcabrera | Take care everyone. | 21:59 |
| *** cppcabrera has quit IRC | 21:59 | |
| *** kgriffs is now known as kgriffs_afk | 21:59 | |
| *** kgriffs_afk is now known as kgriffs | 22:12 | |
| *** kgriffs is now known as kgriffs_afk | 22:13 | |
| *** oz_akan_ has joined #openstack-marconi | 22:21 | |
| *** oz_akan_ has quit IRC | 22:25 | |
| *** amitgandhi has quit IRC | 22:35 | |
| *** amitgandhi has joined #openstack-marconi | 23:39 | |
| *** oz_akan_ has joined #openstack-marconi | 23:52 | |
| *** oz_akan_ has quit IRC | 23:52 | |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!