*** nikhil has quit IRC | 00:45 | |
*** david-ly_ has joined #openstack-searchlight | 02:39 | |
*** david-lyle has quit IRC | 02:40 | |
*** david-ly_ is now known as david-lyle | 03:17 | |
*** nikhil has joined #openstack-searchlight | 03:22 | |
*** lakshmiS has joined #openstack-searchlight | 04:34 | |
*** david-lyle has quit IRC | 06:58 | |
*** david-lyle has joined #openstack-searchlight | 07:14 | |
*** lakshmiS has quit IRC | 11:35 | |
*** lakshmiS has joined #openstack-searchlight | 11:45 | |
nikhil | TravT sjmc7 rosmaita fyi, https://review.openstack.org/#/c/221307/ | 13:36 |
---|---|---|
nikhil | lakshmiS oops ^ | 13:37 |
lakshmiS | nikhil: yeah oops exact time commit | 13:37 |
nikhil | lakshmiS haha, what did you change? | 13:37 |
lakshmiS | member repo from inside location.py was going through the all the layers including policy for retrieving members | 13:38 |
nikhil | I see | 13:38 |
nikhil | ack | 13:38 |
lakshmiS | but in original code it was just going through location and then db | 13:38 |
nikhil | just noticed that, I made a relatively naive change to it but yours looks much better | 13:39 |
lakshmiS | what was your change | 13:39 |
nikhil | the self.store_api was being set twice | 13:39 |
nikhil | in the original patch so I changed it | 13:39 |
nikhil | lakshmiS | 13:39 |
lakshmiS | yeah i remove that too | 13:39 |
lakshmiS | removed | 13:39 |
nikhil | cool | 13:39 |
lakshmiS | thanks for catch. just reaffirms i am not doing something crazy | 13:44 |
nikhil | :) | 13:48 |
lakshmiS | down to 3 errors now. atleast it shows the errors | 13:48 |
lakshmiS | do you know what this might be. CantStartEngineError: No sql_connection parameter is established | 13:49 |
lakshmiS | https://jenkins05.openstack.org/job/gate-glance-python27/1541/console | 13:49 |
lakshmiS | nikhil: | 13:52 |
*** GB21 has joined #openstack-searchlight | 14:45 | |
*** GB21_ has joined #openstack-searchlight | 14:45 | |
*** TravT has quit IRC | 14:49 | |
*** nikhil has quit IRC | 14:58 | |
*** nikhil has joined #openstack-searchlight | 14:58 | |
*** GB21_ has quit IRC | 15:01 | |
*** TravT has joined #openstack-searchlight | 15:02 | |
*** TravT has quit IRC | 15:17 | |
lakshmiS | nikhil: i think i found the next issue. lets see the patch tests run | 15:20 |
nikhil | lakshmiS thanks! | 15:28 |
lakshmiS | nikhil: py27 finally passed. time for some serious reviews on location.py code | 15:35 |
lakshmiS | manually i only tested image member CRUD. not sure how to test location/store code | 15:36 |
*** TravT has joined #openstack-searchlight | 15:36 | |
lakshmiS | image member CRUD and notifications | 15:36 |
nikhil | lakshmiS umm, I rhink we will need to test the image data after CRUD on the image | 15:42 |
nikhil | basically, I am trying to grasp what the store_api is needed for .. to generate image notifications | 15:42 |
openstackgerrit | Steve McLellan proposed openstack/searchlight: Fix sorting behavior for common problematic fields https://review.openstack.org/226378 | 15:43 |
nikhil | if the image data is deleted on image member delete then we have some serious issue | 15:43 |
nikhil | however, if it's just manipulating the access to that data -- it's okay | 15:44 |
lakshmiS | notifications itself dont need them | 15:44 |
nikhil | yeah, I figured it must be the domain model that's enforcing you to impl it so | 15:44 |
lakshmiS | but to seperate image_members as its own gateway layer i added to the layer list since it has other methods in it glance.location.ImageMemberRepoProxy | 15:44 |
nikhil | I see | 15:46 |
lakshmiS | glance.location.ImageMemberRepoProxy._set_acls is called for member add and remove functions | 15:46 |
*** TravT has quit IRC | 15:46 | |
nikhil | and do you think we absolutely need the store api there? | 15:46 |
lakshmiS | it was already there and i just seperated it to its own layer model | 15:47 |
lakshmiS | the only difference between the old code and the new patch in location.py is --- instead of calling get_member_repo on image, it now gets its own member_repo proxy | 15:48 |
*** TravT has joined #openstack-searchlight | 15:48 | |
nikhil | I understand the change better now | 15:50 |
nikhil | you have separated it at the gateway layer | 15:50 |
lakshmiS | yes | 15:50 |
lakshmiS | similar to other repo's | 15:50 |
nikhil | and moved the static method in API to rather use from that gateway | 15:50 |
lakshmiS | yes | 15:51 |
nikhil | and i understand that you needed to do that for separating ti from images | 15:51 |
nikhil | guess, that's where the notification proxy was not getting wrapped for members | 15:51 |
lakshmiS | yes | 15:51 |
nikhil | awesome, thanks! | 15:51 |
nikhil | I think we should be okay store wise | 15:51 |
lakshmiS | that was tricky without separting it as its own repo | 15:52 |
nikhil | Hemanth is testing it | 15:52 |
nikhil | def | 15:52 |
lakshmiS | great. thanks | 15:52 |
nikhil | did they work locally for you? | 15:52 |
nikhil | I mean tests | 15:52 |
lakshmiS | i tested image_member CRUD and notifications | 15:52 |
lakshmiS | and all tests passed | 15:53 |
nikhil | can you please run the whole suite once? | 15:53 |
nikhil | he was sorta worried that it will take forever to check at the gate | 15:53 |
lakshmiS | sure i can. zuul already passed on py27 and py34 | 15:53 |
nikhil | ah cool, please check locally too | 15:54 |
lakshmiS | ok. will let you know once its done locally | 15:54 |
nikhil | thanks@ | 15:55 |
*** TravT has quit IRC | 16:00 | |
lakshmiS | nikhil: local py27 and py34 passed. anything else your run beyond that? | 16:20 |
nikhil | lakshmiS no, thank you very much! | 16:20 |
nikhil | waiting for jenkins | 16:21 |
lakshmiS | cool | 16:21 |
*** TravT has joined #openstack-searchlight | 16:24 | |
*** TravT has quit IRC | 16:28 | |
*** TravT has joined #openstack-searchlight | 16:42 | |
*** david-lyle has quit IRC | 16:46 | |
sjmc7 | TravT - for https://bugs.launchpad.net/searchlight/+bug/1496464.. you're saying do the same thing as e.g. the nova api? | 16:49 |
openstack | Launchpad bug 1496464 in OpenStack Search (Searchlight) "Admin rbac filtering too lenient " [Undecided,New] | 16:49 |
TravT | perhaps. that's the initial idea i have. | 16:50 |
TravT | do you have other ideas? | 16:50 |
TravT | is there another project around that's actually implemented domains properly that we should consider as well? | 16:51 |
sjmc7 | you can still achieve that by filtering on tenant | 16:51 |
sjmc7 | not that i know of | 16:51 |
TravT | i know you can achieve filtering by tenant from query, | 16:51 |
sjmc7 | so it's a usability question then | 16:52 |
TravT | That means a couple things: | 16:52 |
sjmc7 | what's the common case as an admin? to see everything? or only your stuff | 16:52 |
TravT | 1) Every client will have to figure out their token scoping before making a query | 16:52 |
TravT | 2) Every client will have to know the proper query for each resource type to then applying query | 16:53 |
TravT | as an admin in horizon, i typically and logged into a certain project. | 16:53 |
sjmc7 | yeah, you have to | 16:54 |
TravT | when i'm in the project dashboard, all the data returned should be scoped to my currently scoped project | 16:54 |
TravT | when i'm in the admin dashboard, i should be able to see across projects. | 16:54 |
TravT | for images it is more complex | 16:54 |
TravT | because of membership | 16:55 |
sjmc7 | ok. i don't have particularly strong feelings either way. i think it'll get a bit muddy if there's a global search box | 16:55 |
TravT | yeah, that's definitely true. | 16:55 |
TravT | my leaning is to be more restrictive by default | 16:55 |
sjmc7 | no.. we just dont' treat admins as special unless the magic all_projects flag is set | 16:55 |
sjmc7 | i don't think it'll be complex for images | 16:55 |
TravT | has any project besides keystone implemented domains? | 16:58 |
TravT | if david-lyle was online, i'd ask him.... | 16:59 |
TravT | maybe dan is around? | 16:59 |
sjmc7 | not that i'm aware of | 16:59 |
sjmc7 | heat has, a bit | 16:59 |
sjmc7 | but primarily for internal use | 16:59 |
TravT | all_projects seems pretty harmless... as long as it doesn't back us into a corner | 17:00 |
nikhil | lakshmiS just commentec | 17:43 |
nikhil | lakshmiS I think it's worth a new PS, but I can quickly +2 it and let hemanth know too | 17:43 |
lakshmiS | sure. thanks i will put the modified patchset | 17:46 |
nikhil | thanks | 17:47 |
*** GB21 has quit IRC | 18:18 | |
openstackgerrit | Steve McLellan proposed openstack/searchlight: Standardize created_at, updated_at https://review.openstack.org/226524 | 19:47 |
*** rosmaita has quit IRC | 20:29 | |
*** rosmaita has joined #openstack-searchlight | 20:33 | |
lakshmiS | nikhil, rosmaita: thanks for the reviews | 20:34 |
*** lakshmiS has quit IRC | 20:37 | |
*** TravT has quit IRC | 20:45 | |
openstackgerrit | Steve McLellan proposed openstack/searchlight: Add faceting https://review.openstack.org/222388 | 20:50 |
openstackgerrit | Steve McLellan proposed openstack/searchlight: Add faceting https://review.openstack.org/222388 | 20:54 |
openstackgerrit | Steve McLellan proposed openstack/searchlight: Add faceting https://review.openstack.org/222388 | 20:59 |
openstackgerrit | Steve McLellan proposed openstack/searchlight: Standardize created_at, updated_at https://review.openstack.org/226524 | 21:00 |
*** TravT has joined #openstack-searchlight | 21:50 |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!