Added solr route cookie for solr-service requests

[marblestation]

When a request is directed to solr-service, the user token is extracted from the request and its solr route (which link to a specific solr instance) is retreived from redis (if it exists) before passing the request to solr-service.

Solr-service response is also inspected for "Set-Cookie" instructions with the solr route, in case it is present the solr route is stored in redis using the user's token as key.

[coveralls]

Coverage decreased (-0.4%) to 82.655% when pulling d60a35d on marblestation:set_solr_route into 2b3655b on adsabs:master.

[romanchyla]

great job!

please see one question and one request; it is IMHO good to go - but the gap for the remote call should be covered

[romanchyla]

what is the solr-service is ran in the remote mode? Maybe also need to add block in the remote_app_context?

[romanchyla]

occurred to me that this depends on the oauth decorator running first (which is the case), however as it stands right now, it is conditional:

https://github.com/marblestation/adsws/blob/1013364babcb059dd7ec5daaa1ff31fc83ea55e3/adsws/api/discoverer/utils.py#L81

which might be OK (for now); but I have got a question for ya (since you might have been digging inside); when I am authenticated through a session cookie (ie. not sending the token), is the user object (and oauth info) loaded before your sroute gets called?

[marblestation]

I have done some testing and we cannot actually make a request to solr-service only with a session cookie, it requires the bearer token or we just get "Unauthorized".

[romanchyla]

while that is probably true; there are endpoints, such as /user where an user is granted access with just a cookie; and so if this function expects user_token we should verify that it is there

[marblestation]

But the solr_route decorator is only for the solr-service, requests to /user do not go through the solr_route decorator. Do you know a microservice that grants access just with a session cookie and that contact solr-service for something?

[romanchyla]

@marblestation i can't agree with the latest change - the /search is IMO worse than the module path (and can be changed via config)

but the most important thing was the remote context; that seems to be not addressed at all; unless i'm not seeing all commits: https://github.com/marblestation/adsws/blob/491f9f60b7c0238c1d885750b730cdfdbefb21f2/adsws/api/discoverer/utils.py#L95

[marblestation]

The remote context was addressed here: https://github.com/marblestation/adsws/blob/491f9f60b7c0238c1d885750b730cdfdbefb21f2/adsws/api/discoverer/utils.py#L178

And actually that's what motivated the change to deploy_path == '/search', for remote services we do not have package names. I thought it would be good to have both (local and remote) to use the same criteria, and I thought deploy_path was a good way to go because if in the future we move solr-service from a local to a remote service, we may keep the same URL. What is your preferred solution? Should I change back the condition for the local service? I don't really have a strong opinion on this.

[romanchyla]

ok, sorry, here is the explanation - i was taking a mental shortcut

first, the local/remote handling look identical - and I didn't notice that the remote was there, my bad. secondly, and that became clear to me only now: I was thinking "the mechanism should be generic".

If you notice, we are hardcoding a tight coupling right into the heart of adsws (which escaped me at first, but now I see it clearly) -- adsws shouldn't know/care if it is running /search or treat some module specially

So I'd say instead of

if deploy_path == '/search':
     view = solr_view......

you do:

if deploy_path in current_app.config.get('AFFINITY_ENHANCED_ENDPOINTS', []):
    view = affinitiy_view(AFFINITY_PREFIX=deploy_path,....)(view)

[marblestation]

I agree with your suggestion, I tried to implement something in those lines. What do you think now?