Page MenuHomePhabricator

[services][commtest] Replace gRPC Blob client with HTTP
ClosedPublic

Authored by bartek on Jun 30 2023, 10:09 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Nov 23, 2:29 PM
Unknown Object (File)
Sat, Nov 23, 2:28 PM
Unknown Object (File)
Sat, Nov 23, 2:28 PM
Unknown Object (File)
Mon, Nov 18, 2:43 PM
Unknown Object (File)
Sat, Nov 16, 11:28 PM
Unknown Object (File)
Sat, Nov 16, 11:28 PM
Unknown Object (File)
Sat, Nov 16, 11:28 PM
Unknown Object (File)
Sat, Nov 16, 11:28 PM
Subscribers

Details

Summary

Resolves ENG-3869. Decided to finish this now to unblock gRPC removal and leave Commtest unbroken when using the new Blob service DB schema that will require the new HTTP interface.

A brief summary of the changes:

  • Added reqwest dependency - a HTTP client. Also added serde but we already use it in other services - added @ashoat as a reviewer
  • Replaced gRPC BlobServiceClient utilities with HTTP reqwest calls.
  • Removed some unnecessary &mut references

This diff is pretty big but I had difficulties splitting it into smaller ones. I added inline comments to explain the changes.

Test Plan

Started Blob service in trace log level, with the following command:

RUST_LOG=blob=trace cargo run -- --sandbox --localstack-url "http://localstack:4566" --http-port 50053

Important thing is to set the port to 50053 because this is what the commtest expects - it reads the COMM_SERVICES_PORT_BLOB env var that is set in services/.env

Then started both integration and performance tests:

cd services
yarn run-integration-tests blob
yarn run-performance-tests blob

Observed blob service trace logs and confirmed it is doing steps that are expected for the tests.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bartek held this revision as a draft.
bartek published this revision for review.Jun 30 2023, 10:34 PM
bartek added 2 blocking reviewer(s): ashoat, michal.
bartek added inline comments.
services/commtest/Cargo.toml
29–30 ↗(On Diff #28326)

cc @ashoat:

The reqwest is a HTTP client library. It is double-licensed: MIT or Apache 2.0

Serde is already used in other services

Dependency looks good

Looks good, we could potentially use reqwest:Url and .set_port(), .join() for URL manipulation.

This revision is now accepted and ready to land.Jul 3 2023, 3:50 AM

Rebase. Use reqwest::Url instead of strings