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

feature(bigtable): added bigtable client creation #508

Closed
wants to merge 7 commits into from

Conversation

zZHorizonZz
Copy link
Contributor

@zZHorizonZz zZHorizonZz commented Sep 20, 2023

This PR introduces an experimental version of the Bigtable client creation.

Summary:

  • Introduced a BigtableClient annotation that qualifies an injected Bigtable client.
  • The BigtableClientProcessor class is responsible for registering beans, discovering injected beans, generating bigtable client producers, and transforming injection points. This ensures the right Bigtable client is used based on the configuration and qualifiers.
  • The main class BigtableClients handles the creation of the Bigtable client.
  • The system for bean creation is based on grpc client bean creation.

Also configuration will be made so that you can configure it like this:

quarkus.google.cloud.bigtable.clients.test-client.instance-id=test-instance

or default client:

quarkus.google.cloud.bigtable.clients.instance-id=test-instance

The introduced Bigtable client creation is experimental and may be subject to changes based on further testing.

@zZHorizonZz
Copy link
Contributor Author

@loicmathieu When you have a bit of time, could you take a look at this? If the system this PR introduces is okay, can I potentially move forward or should I rework it?

Copy link
Collaborator

@loicmathieu loicmathieu left a comment

Choose a reason for hiding this comment

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

Hi,
Thanks for the contribution.
Is there a strong need for multiple client support? It complexifies a lot the PR where a simple approach with a CDI producer would have worked if only one instance is published.

@zZHorizonZz
Copy link
Contributor Author

Is there a strong need for multiple client support? It complexifies a lot the PR where a simple approach with a CDI producer would have worked if only one instance is published.

So, yeah, that's why I wanted to wait first since you mentioned in #504 (comment) something about multiple clients, etc. Also, I'm not sure if it's normal to use multiple Bigtable instances or just one in production. In any case, I will close this PR and create a new one with just a simple producer. 👍

@loicmathieu
Copy link
Collaborator

Yes, thanks, this would be easier to maintant!

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