all: remove gob and SSE rpc endpoints #758
Conversation
We now only consume zoekt via gRPC at Sourcegraph and I doubt anyone uses the old endpoints. This will have one required update in sourcegraph, and that is to use SenderFunc from the main zoekt package rather than from the now deleted stream package. Test Plan: go test
|
@dgruzd @DylanGriffith which API endpoints do you use in zoekt-webserver? Safe to remove these? |
@keegancsmith We use the Edit: I've also tested it locally with this branch. Everything seems to be ok. Thank you! |
web/e2e_test.go
Outdated
| @@ -964,6 +962,8 @@ func TestHealthz(t *testing.T) { | |||
| } | |||
|
|
|||
| func TestRPC(t *testing.T) { | |||
| t.Skip("TODO grpc and jsonrpc") | |||
There was a problem hiding this comment.
Are you planning to update this test before merging?
There was a problem hiding this comment.
No, I can just remove the test but was hoping to stumble on this in the future and then improve it.
There was a problem hiding this comment.
Let's remove it if we think it's not critical and have decent coverage elsewhere!
There was a problem hiding this comment.
This makes sense to me, it seems like a nice clean-up and I don't see problems removing it.
Aside: it's interesting we don't have Zoekt releases, which is the standard way to document + communicate API breaks. But it doesn't feel like we have enough changes and consumers right now that it'd be worth it.
Yeah right now I am just ccing on PRs the two main downstream consumers. I think its unusual for us to make a few breaking changes so close together. I think we can introduce something more formal if we or others start feeling pain. |
We now only consume zoekt via gRPC at Sourcegraph and I doubt anyone uses the old endpoints. We still support the classic rest like API at
/api/which is used by neogrok.This will have one required update in sourcegraph, and that is to use SenderFunc from the main zoekt package rather than from the now deleted stream package.
Test Plan: go test