Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CassandraSinkCluster: pull routing logic into separate function #1631

Merged

Conversation

rukai
Copy link
Member

@rukai rukai commented May 19, 2024

CassandraSinkCluster has a really large send_message method, it should be broken down into multiple smaller functions.
To start with lets pull out the request routing logic into a route_requests method, this matches a function of the same name and purpose in KafkaSinkCluster.

The send_message function is also really poorly named but that should be addressed by moving the contents into Transform::transform which should occur in another PR to keep scope down.

The "Files changed" diff is very confusing and doesnt properly show the moving of the routing logic.
If you look at the individual commits you can see that the routing logic is removed and then readded as a separate function.
I recommend pasting the large chunk of logic that is removed and readded into a diff tool to verify the logic remains identical.

@rukai rukai force-pushed the cassandra_sink_cluster_refactor_route_requests branch from ccced46 to 0c037a2 Compare May 19, 2024 22:33
@rukai rukai force-pushed the cassandra_sink_cluster_refactor_route_requests branch from 0c037a2 to a252ea7 Compare May 19, 2024 22:34
@rukai rukai force-pushed the cassandra_sink_cluster_refactor_route_requests branch from a252ea7 to 6218a9a Compare May 19, 2024 22:36
@rukai rukai changed the title CassandraSinkCluster: pull routing logic into seperate function CassandraSinkCluster: pull routing logic into separate function May 19, 2024
Copy link

codspeed-hq bot commented May 19, 2024

CodSpeed Performance Report

Merging #1631 will not alter performance

Comparing rukai:cassandra_sink_cluster_refactor_route_requests (31aa0ee) with main (618638a)

Summary

✅ 37 untouched benchmarks

@rukai rukai requested a review from conorbros May 19, 2024 23:02
@rukai rukai merged commit 3cdefda into shotover:main May 21, 2024
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants