| openstackgerrit | Steve McLellan proposed openstack/searchlight: Add 'sort' parameter https://review.openstack.org/206268 | 00:17 |
|---|---|---|
| *** openstackgerrit has quit IRC | 00:46 | |
| *** openstackgerrit has joined #openstack-searchlight | 00:47 | |
| *** sigmavirus24 has quit IRC | 02:07 | |
| *** lakshmiS has joined #openstack-searchlight | 06:46 | |
| TravT | krykowski: if you have a chance, can you look at these patches: https://review.openstack.org/206242 & https://review.openstack.org/#/c/202392/ | 11:24 |
| *** lakshmiS has quit IRC | 13:48 | |
| *** sigmavirus24_awa has joined #openstack-searchlight | 13:57 | |
| *** sigmavirus24_awa is now known as sigmavirus24 | 13:57 | |
| TravT | sigmavirus24: would you be able to look at this one again? https://review.openstack.org/#/c/206242/ | 16:04 |
| sigmavirus24 | TravT: took a quick look but I don't have time to do a deep dive on it so I left a Q | 16:07 |
| TravT | Ok, thanks! | 16:08 |
| sigmavirus24 | Depending on the answer to the Q I can +2 it or -1 =P | 16:09 |
| TravT | well, we'll see what sjmc7 says. The spot you put the question on is what is called during a regular indexing call (not via notifications) | 16:13 |
| TravT | eg searchlight-manage index sync call | 16:13 |
| sjmc7 | my ears are burning | 16:14 |
| sjmc7 | good catch sigmavirus24, thought i'd removed that | 16:14 |
| sigmavirus24 | ah | 16:14 |
| sjmc7 | one sec, will push up a patch directly | 16:14 |
| sigmavirus24 | TravT: see I thought it might be something like that | 16:14 |
| sjmc7 | :) | 16:15 |
| sigmavirus24 | But I wasn't sure because everything else was changes to use the "notification" instead of "image" function | 16:15 |
| sigmavirus24 | so | 16:15 |
| * sigmavirus24 shrugs | 16:15 | |
| * sigmavirus24 just asks questions that are sometimes insightful | 16:15 | |
| TravT | it is always good to ask questions... but now I'm wondering about the flow order. | 16:15 |
| sigmavirus24 | heh | 16:16 |
| * sigmavirus24 sits backs and relaxes all cool | 16:16 | |
| TravT | might have to step through it in the debugger again, because i would have thought it was line 81 that would go away not line 63 | 16:16 |
| openstackgerrit | Steve McLellan proposed openstack/searchlight: Glance image notification serialization fix https://review.openstack.org/206242 | 16:17 |
| sjmc7 | well, now's the time to ask. removed those lines | 16:18 |
| sigmavirus24 | sjmc7: https://github.com/openstack/searchlight/blob/55471cb2f18a7c54bf0ee627191d58bf23ca33b5/searchlight/elasticsearch/plugins/glance/images.py#L123 | 16:19 |
| sigmavirus24 | that might need the is_public stuff | 16:19 |
| TravT | Thanks, sjmc7, I'll run it through its paces... I am going to run debug through both notification flow and index flow... but I think I see why I was confused on that. | 16:19 |
| * sigmavirus24 did a quick github search since TravT brought it up | 16:19 | |
| sjmc7 | the API returns 'visibility' | 16:19 |
| sigmavirus24 | Ah | 16:19 |
| sjmc7 | although... i wonder about v1 images | 16:19 |
| sigmavirus24 | We're only using v2 images API? | 16:19 |
| sigmavirus24 | v1 uses is_public | 16:19 |
| sigmavirus24 | Whatever we do, we should leave a comment explaining it | 16:19 |
| sigmavirus24 | =P | 16:20 |
| sigmavirus24 | or make a separate util function so we aren't copying a ternary around | 16:20 |
| * sigmavirus24 hides again to figure out this keystone v3/swift glance store thing | 16:20 | |
| TravT | that sounds like loads of fun | 16:20 |
| TravT | BTW, sjmc7 as we start looking at this from horizon patches, I have a withering patch I was working on with Julie Pichon to support transition horizon from v1 to v2 glance. | 16:26 |
| TravT | https://review.openstack.org/#/c/150084/4/openstack_dashboard/api/glance.py | 16:26 |
| sjmc7 | sorry, modem reboot | 16:26 |
| sjmc7 | v1 <-> v2 compat is the subject of another patch i think | 16:27 |
| TravT | but I do believe what you have done means v1 won't work for indexing. | 16:27 |
| sjmc7 | i doubt this is the only thing | 16:27 |
| sjmc7 | i haven't changed anything for v1 :) | 16:27 |
| TravT | no you did it | 16:27 |
| sjmc7 | if v1's broken, it's broken | 16:27 |
| sjmc7 | i did many things but not that | 16:27 |
| * TravT just wanted to blame sjmc7 | 16:27 | |
| sjmc7 | this osunds like a Meatloaf song | 16:27 |
| TravT | ok, i have about 40 minutes. are you thinking you'll do more with that patch or not? if so, i'll spend my time on other things. if so, I will run it through its paces again. | 16:29 |
| TravT | 2nd if so should be if not | 16:29 |
| sjmc7 | so yes, it sounds like v1 may be unhappy. but i don't think that's made better or worse by this patch - this *should* only affect notifications which apparently are always v1. well, the only thing is whether i change it to always call the API | 16:29 |
| TravT | you know | 16:29 |
| sjmc7 | as we briefly discussed yesterday. and maybe that's better | 16:29 |
| TravT | i do think in the long run that will be better. | 16:30 |
| sjmc7 | ok. i can certainly do that | 16:30 |
| sjmc7 | in which case, do something else for your 40 minutes | 16:30 |
| TravT | but, we could make that an item of discussion on thursday's meeting when we have more attention | 16:31 |
| TravT | i'd like to get a working glance listener | 16:31 |
| sjmc7 | yeah. maybe let's get this in and then can rework later | 16:31 |
| sjmc7 | since right now it's broken out of the box | 16:31 |
| TravT | i would not complain if the visibility / public was its own function | 16:31 |
| TravT | whether or not the regular serialize function calls it isn't totally necessary at this point | 16:32 |
| sjmc7 | if i do that i might as well call it there too | 16:32 |
| sjmc7 | i can make that change leter | 16:32 |
| TravT | ok, well, i'll put this patchset through its paces. | 16:35 |
| sjmc7 | gimme two secs and i'll put that change up | 16:35 |
| TravT | ok. | 16:35 |
| TravT | I'll be back in a couple minutes myself then. | 16:35 |
| openstackgerrit | Steve McLellan proposed openstack/searchlight: Glance image notification serialization fix https://review.openstack.org/206242 | 16:56 |
| sjmc7 | sorry TravT, my turn for a bad ISP | 16:57 |
| sjmc7 | patch is up | 16:58 |
| TravT | sjmc7, just looking now | 16:59 |
| TravT | i like that its pulled out. | 16:59 |
| TravT | i guess no discrete unit test since it is _function? | 16:59 |
| sjmc7 | yeah | 16:59 |
| TravT | okay, i was just going to start hacking at another patch, but I'll spend the next ten minutes running this instead and see where I get. | 17:00 |
| sjmc7 | the notification unit test (and any v1 format test we don't have) should cover it | 17:00 |
| TravT | although there is not expected of "visibility" : "private" | 17:01 |
| sjmc7 | i can put another test in | 17:01 |
| TravT | oh wait, maybe there is in the other tests in that file? | 17:01 |
| sjmc7 | there are some private tests, yeah. thiough they start with a v2-like input | 17:02 |
| TravT | probably better to go ahead and add a notification test where is_public starts false. | 17:02 |
| sjmc7 | ok dokey. give it a test as-is, i won't change anything functionally | 17:03 |
| TravT | i'm getting pinged in hipchat for stuff now... :S | 17:04 |
| sjmc7 | :) | 17:04 |
| TravT | this is what i get for logging back into hipchat | 17:06 |
| TravT | sjmc7, i'll be out for the next hour or so. but will go through validating that patch as soon as I'm back. | 17:14 |
| sjmc7 | sure thing | 17:14 |
| sigmavirus24 | sjmc7: TravT do you know lakshmi? | 18:04 |
| sjmc7 | yep | 18:04 |
| sigmavirus24 | Could you have him look at https://review.openstack.org/195775 once more? | 18:04 |
| sigmavirus24 | Eric responded, and I think we're good to go, but lakshmi and krykowski are SMEs | 18:05 |
| sigmavirus24 | oh krykowski could you look at https://review.openstack.org/195775 too? | 18:05 |
| sjmc7 | mailed him, but he's in India so likely won't be til tomorrow | 18:06 |
| sigmavirus24 | sjmc7: no worries | 18:19 |
| sigmavirus24 | Not a rush | 18:19 |
| sigmavirus24 | It just has a +2 and I wanted to make sure his concern(s) were addressed by the reply | 18:19 |
| sjmc7 | very courteous! | 18:20 |
| openstackgerrit | Steve McLellan proposed openstack/searchlight: Glance image notification serialization fix https://review.openstack.org/206242 | 20:50 |
| TravT | sjmc7: ^ does that mean I should test out the patch now? | 20:51 |
| TravT | anything more to come? | 20:51 |
| sjmc7 | have you been waiting all afternoon?? | 20:51 |
| sjmc7 | no, i'm done | 20:51 |
| sjmc7 | it was a good suggestion | 20:51 |
| TravT | ok. I wasn't explicitly waiting for your patch. I got pulled into several other things. | 20:52 |
| sjmc7 | just yankin your chain. i think it's done, and i also think on reflection it would be better to switch to use the API for now, at least until notifications give us everything | 20:52 |
| TravT | i still think notification payload will be a problem in general | 20:54 |
| TravT | but, let's look through this one. | 20:54 |
| TravT | btw, i found a fun way to confuse yourself with notifications earlier | 20:55 |
| TravT | set up glance to publish to notifications, searchlight_indexer topic | 20:56 |
| TravT | then have searchlight_listener service running | 20:56 |
| TravT | and start up searchlight listener in pycharms debugger | 20:56 |
| TravT | then watch your hair fall to the ground as you try to figure out why your debugger breakpoints don't seem to affect the index as you cross reference via elastic search direct queries | 20:57 |
| sigmavirus24 | TravT: I don't even understand that | 21:03 |
| sigmavirus24 | then again I use vim | 21:04 |
| sigmavirus24 | so | 21:04 |
| sigmavirus24 | I've already lost all my hair | 21:04 |
| sigmavirus24 | If you want to pull your hair out | 21:04 |
| sigmavirus24 | Use Glance + Keystone v3 + Swift and watch as nothing gets logged from glance_store about the auth failure and you get an error that makes it seem like nothing even reached glance_store yet from glance v1 api | 21:05 |
| TravT | ooh, that seems like a great way to lose your hair. | 21:05 |
| sigmavirus24 | Oh yeah | 21:09 |
| sigmavirus24 | Then I remembered Jamie Lennox had a patch fixing it | 21:09 |
| sigmavirus24 | Then I didn't look closely enough and it still didn't work | 21:09 |
| sigmavirus24 | Then i read his patch very closely | 21:10 |
| sigmavirus24 | And saw that there are new config options I have to set | 21:10 |
| sigmavirus24 | And then I got it working | 21:10 |
| TravT | lovely | 21:14 |
| TravT | I hope you requested better documentation. | 21:15 |
| TravT | sigmavirus24: I just locally reverified the serialization fix. When you are able to look again, would appreciate it. https://review.openstack.org/#/c/206242/ | 21:57 |
| TravT | sjmc7 ^ | 21:57 |
| sigmavirus24 | One comment in-line but it's not really important for that patch to be approved (so I approved it) | 22:01 |
| openstackgerrit | Merged openstack/searchlight: Glance image notification serialization fix https://review.openstack.org/206242 | 22:06 |
| sigmavirus24 | fast gate is fast | 22:07 |
| TravT | wow | 22:07 |
| TravT | i walked away to get a drink of water, come back and it merged. | 22:08 |
| * sigmavirus24 has many things in the air at this moment | 22:08 | |
| sigmavirus24 | I left the actual testing to you TravT | 22:08 |
| TravT | good comment | 22:08 |
| sigmavirus24 | I just looked at the code | 22:08 |
| sigmavirus24 | I trust you tested it thoroughly | 22:08 |
| TravT | testing it has consumed a good portion of my day. | 22:08 |
| sigmavirus24 | I've been making glance + swift + keystone v3 work for most of the day | 22:08 |
| sigmavirus24 | when I wasn't participating remotely in glance midcycle discussions | 22:09 |
| sigmavirus24 | or digging into cryptography | 22:09 |
| sigmavirus24 | or ... crying | 22:09 |
| TravT | :P | 22:09 |
| TravT | i found a few error handling items while testing this | 22:09 |
| TravT | not related to this patch directly | 22:10 |
| TravT | that I might put up a patch to start a conversation about. | 22:10 |
| openstackgerrit | Travis Tripp proposed openstack/searchlight: Change Glance notifications to be create_or_update https://review.openstack.org/206756 | 22:52 |
| *** sigmavirus24 is now known as sigmavirus24_awa | 23:07 | |
| openstackgerrit | Travis Tripp proposed openstack/searchlight: Glance index visibility should only be for v1 https://review.openstack.org/206762 | 23:35 |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!