refactor: Remove SQS references from Coordinator #365
Conversation
| ) -> Option<JoinHandle<i64>> { | ||
| let redis_connection_manager = redis_connection_manager.clone(); | ||
| let s3_client = s3_client.clone(); |
There was a problem hiding this comment.
Why do we clone these instead of using the referenced client directly?
There was a problem hiding this comment.
tokio::spawn requires, and takes, ownership of the variables referenced, this is signified by the move keyword. The compiler does not know how long the async task (tokio::spawn) will last, and therefore cannot determine whether the reference will live long enough.
Cloning allows us to take ownership of the variable, which then means we can move it to the async task.
| if error | ||
| .downcast_ref::<aws_sdk_s3::error::NoSuchKey>() | ||
| .is_some() | ||
| { |
There was a problem hiding this comment.
Is this code changed to catch more errors?
There was a problem hiding this comment.
Nah it's just a clippy suggestion - it behaves the same
The docker image doesn't reference AWS directly, but I assume you are referring to the configuration of the the docker container in GCP? I'll eventually raise another PR to remove that, as well as the terraform config for both GCP and AWS. For now, they'll just be silently ignored. |
|
Sweet! Thanks for taking care of all the clippy suggestions too! |
Doing some clean-up to make the development of #200 a bit easier. Now that we have moved completely to Redis, this PR removes all SQS code/references from Coordinator.
Additionally, I did some small refactoring to
QueryApiContext, adding boths3_clientandjson_rpc_client, allowing a single instance of each to be passed around and used throughout the code. Previously, manys3_clients were being instantiated in various parts of the code. This removed the dependancy onOptsfrom Historical Backfill, so that has also been removed.Finally, I fixed all Clippy errors :)