Remove HTTP for inter-service RPC by eseliger · Pull Request #59093 · sourcegraph/sourcegraph-public-snapshot

eseliger · 2023-12-18T23:04:30Z

In the upcoming release, we will only support gRPC going forward. This PR removes the old HTTP client and server implementations and a few leftovers from the transition.

Closes https://github.com/sourcegraph/sourcegraph/issues/59494

Test plan

CI and manual testing.

eseliger · 2023-12-18T23:55:24Z

eseliger · 2023-12-19T22:07:13Z

was missing in gRPC but existed in HTTP. Found it while converting existing tests that exercised HTTP only.

cc @camdencheek @sourcegraph/search-platform FYI, found this while porting the HTTP test to gRPC.

eseliger · 2023-12-19T22:08:55Z

This is covered by the repoLookup test at the end of this file, no good use converting it to gRPC as there's no logic happening in the transport layer

eseliger · 2023-12-19T22:11:37Z

We no longer expose an HTTP API so this is not needed anymore. CCing @sourcegraph/security-code-review just for visibility.

eseliger · 2023-12-19T22:13:01Z

moved to repo_info_test.go where the implementation of repoDelete lives, now tested without the transport layer in between.

In the upcoming release, we will only support gRPC going forward. This PR removes the old HTTP client and server implementations and a few leftovers from the transition.

sourcegraph-bot · 2024-01-10T19:18:00Z

Codenotify: Notifying subscribers in CODENOTIFY files for diff 53a48d9...a0aa93e.

Notify	File(s)
@bobheadxi	monitoring/definitions/embeddings.go monitoring/definitions/frontend.go monitoring/definitions/git_server.go monitoring/definitions/precise_code_intel_worker.go monitoring/definitions/repo_updater.go monitoring/definitions/searcher.go monitoring/definitions/shared/BUILD.bazel monitoring/definitions/shared/frontend.go monitoring/definitions/symbols.go monitoring/definitions/worker.go
@jtibshirani	internal/search/alert/BUILD.bazel internal/search/alert/observer.go internal/search/backend/BUILD.bazel internal/search/backend/grpc.go internal/search/backend/horizontal.go internal/search/backend/zoekt.go internal/search/commit/commit.go internal/search/job/jobutil/BUILD.bazel internal/search/job/jobutil/alert.go internal/search/job/jobutil/exhaustive_job.go internal/search/job/jobutil/filter_file_contains.go internal/search/job/jobutil/filter_file_contains_test.go internal/search/job/jobutil/repo_pager_job.go internal/search/job/jobutil/repos.go internal/search/repos/BUILD.bazel internal/search/repos/repos.go internal/search/repos/repos_test.go internal/search/searcher/BUILD.bazel internal/search/searcher/client.go internal/search/searcher/client_grpc.go internal/search/searcher/search.go internal/search/searcher/stream.go internal/search/structural/structural.go
@keegancsmith	cmd/searcher/internal/search/BUILD.bazel cmd/searcher/internal/search/hybrid_test.go cmd/searcher/internal/search/search.go cmd/searcher/internal/search/search_grpc.go cmd/searcher/internal/search/search_test.go cmd/searcher/internal/search/store_test.go cmd/searcher/shared/BUILD.bazel cmd/searcher/shared/shared.go cmd/symbols/internal/api/BUILD.bazel cmd/symbols/internal/api/handler.go cmd/symbols/internal/api/handler_cgo.go cmd/symbols/internal/api/handler_nocgo.go cmd/symbols/internal/api/handler_test.go cmd/symbols/squirrel/BUILD.bazel cmd/symbols/squirrel/http_handlers.go internal/search/alert/BUILD.bazel internal/search/alert/observer.go internal/search/backend/BUILD.bazel internal/search/backend/grpc.go internal/search/backend/horizontal.go internal/search/backend/zoekt.go internal/search/commit/commit.go internal/search/job/jobutil/BUILD.bazel internal/search/job/jobutil/alert.go internal/search/job/jobutil/exhaustive_job.go internal/search/job/jobutil/filter_file_contains.go internal/search/job/jobutil/filter_file_contains_test.go internal/search/job/jobutil/repo_pager_job.go internal/search/job/jobutil/repos.go internal/search/repos/BUILD.bazel internal/search/repos/repos.go internal/search/repos/repos_test.go internal/search/searcher/BUILD.bazel internal/search/searcher/client.go internal/search/searcher/client_grpc.go internal/search/searcher/search.go internal/search/searcher/stream.go internal/search/structural/structural.go internal/symbols/BUILD.bazel internal/symbols/client.go
@slimsag	monitoring/definitions/embeddings.go monitoring/definitions/frontend.go monitoring/definitions/git_server.go monitoring/definitions/precise_code_intel_worker.go monitoring/definitions/repo_updater.go monitoring/definitions/searcher.go monitoring/definitions/shared/BUILD.bazel monitoring/definitions/shared/frontend.go monitoring/definitions/symbols.go monitoring/definitions/worker.go
@varungandhi-src	cmd/symbols/squirrel/BUILD.bazel cmd/symbols/squirrel/http_handlers.go

pjlast

Well I've looked through most of this PR and nothing problematic jumps out at me 😅 a diff this large is obviously tricky though.

Also checked it out locally and played around and didn't encounter any shenanigans.

eseliger · 2024-01-11T18:46:22Z

I am going to merge this, happy to address any additional feedback! It's probably prudent to do that in follow-up PRs given this diff is already massive 😬

cla-bot Bot added the cla-signed label Dec 18, 2023

github-actions Bot added the team/source Tickets under the purview of Source - the one Source to graph it all label Dec 18, 2023

eseliger force-pushed the es/grpc-cleanup branch from c4b2d8f to c15922c Compare December 18, 2023 23:38

eseliger commented Dec 18, 2023

View reviewed changes

Comment thread internal/gitserver/proxy.go Outdated

eseliger Dec 18, 2023

Copy link
Copy Markdown

Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unused

eseliger commented Dec 18, 2023

View reviewed changes

Comment thread internal/gitserver/client.go Outdated