search

google / zoekt

Fast trigram based code search
1.67k stars 113 forks source link

Revive REST functionality with what conditions? #126

Open robinp-tw opened 3 years ago

robinp-tw commented 3 years ago

Hello,

I found there was REST search functionality that was deleted in https://github.com/google/zoekt/commit/744484438f16a7a3458cd4175863a982d7b726ea. I find that functionality useful, so would like to restore it.

Can it be restored as-is (maybe with some tweaks, to support result size limit from request etc), or do you have any concerns with it? Thank you!

hanwen commented 3 years ago

The REST support was removed because I added it to support some other functionality which never panned out.

What do you want to do? I see two options:

1) Make the core API available over HTTP / JSON.

2) add a &format=JSON parameter to the Search endpoint in server.go, and dump the data in the web API format.

my main concern is that I don't want to have an extra API flavor, because that necessarily will create extra conversion code that needs to be maintained.

Maybe the srcgraph folks have ideas/preferences too. Summoning @keegancsmith

keegancsmith commented 3 years ago

Maybe the srcgraph folks have ideas/preferences too. Summoning @keegancsmith

We use "net/rpc" to remotely expose a "zoekt.Searcher". We originally used the json API, but quickly moved to something which can expose the core API. What about option 2, but directly marshalling the response from Searcher.Search into JSON?

FYI we have been working on a streaming API internally, and will likely add an adapter to zoekt.Searcher which allows use to do streaming RPCs. I wasn't originally thinking this would be that useful to upstream, but if you feel otherwise we can debate that decision :)

hanwen commented 3 years ago

Is the streaming just for the browser <-> server communications? You'd need all results anyway to sort the results, right?

keegancsmith commented 3 years ago

Is the streaming just for the browser <-> server communications?

browser <-> server. But we also want to do streaming between services so as to return the results sooner. We have use cases where a search can take a long time. For example a poor regular expression over a large cluster can take a while to return.

You'd need all results anyway to sort the results, right?

We currently aren't relying on ranking/sorting, so no. When we do start doing this, we will try and visit shards/repos in a smart order (somewhat like the rankedShard idea in zoekt).

hanwen commented 3 years ago

might make sense to add an option "Unsorted" to the SearchRequest, then making all of the APIs streaming makes more sense.

kisabaka commented 2 years ago

@hanwen @keegancsmith JSON marshalling seems to be a simple change: https://github.com/kisabaka/zoekt/commit/6b51c3c560af34f18a46add47e988dd01448decc

Let me know if you're happy with that approach