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

Migrate Control Plane codegen to quickwit-proto crate #3678

Merged

Conversation

guilload
Copy link
Member

Description

I'm also trying to align naming and packaging conventions between services.

How was this PR tested?

make test-all

pub mod indexing;
#[path = "quickwit/quickwit.metastore.rs"]
#[path = "codegen/quickwit/quickwit.metastore.rs"]
Copy link
Member Author

Choose a reason for hiding this comment

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

At this point, we want some code formatting in quickwit_proto, so I moved all the generated files to a codegen folder.

@guilload guilload requested a review from imotov July 24, 2023 15:22
Copy link
Collaborator

@imotov imotov left a comment

Choose a reason for hiding this comment

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

Really nice clean-up!

It looks like we could now move the service_client_pool from grpc-clients into search crate and get rid of grpc-clients crate. This crate confused me earlier because I didn't realize it is part of the legacy structure. So, maybe we should also deprecate service_client_pool for good measure.

@guilload guilload force-pushed the guilload/migrate-control-plane-proto-to-quickwit-proto branch from 8c2e4cc to 65c85d5 Compare July 24, 2023 17:44
@guilload
Copy link
Member Author

Yes. We might be able to get rid of quickwit-grpc-clients altogether because I've already migrated the searcher pool to the "new" pool. Let me see if we use anything from the ServiceClient trait.

@guilload guilload merged commit 3e97d0a into main Jul 24, 2023
3 checks passed
@guilload guilload deleted the guilload/migrate-control-plane-proto-to-quickwit-proto branch July 24, 2023 17:56
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.

2 participants