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

feat: use concrete classes for hbase.client.registry.impl (#4043) #4188

Merged
merged 6 commits into from
Oct 18, 2023

Conversation

igorbernstein2
Copy link
Collaborator

@igorbernstein2 igorbernstein2 commented Oct 16, 2023

Previously we used runtime bytecode generation which prevented endusers from using hbase-site.xml to target bigtable in hbase 2 applications. This PR will create a concrete implementation for both AsyncRegistry for (hbase < 2.3) and ConnectionRegistry (for hbase >= 2.3).
To accomplish this, we use a local copy of the AsyncRegistry interface from HBase 2.2 that we use as a compile time target and then strip it from the resulting jar.

After this change end users should be able to use async apis in hbase 2 using hbase-site.xml. Examples:

<configuration>
    <property>
        <name>hbase.client.connection.impl</name>
        <value>com.google.cloud.bigtable.hbase2_x.BigtableConnection</value>
    </property>
    <property>
        <name>hbase.client.async.connection.impl</name>
        <value>org.apache.hadoop.hbase.client.BigtableAsyncConnection</value>
    </property>
    <property>
        <name>hbase.client.registry.impl</name>
        <!-- When using hbase >= 2.3, use this -->
        <value>org.apache.hadoop.hbase.client.BigtableConnectionRegistry</value>
        <!-- When using hbase < 2.3, use this -->
        <!-- <value>org.apache.hadoop.hbase.client.BigtableAsyncRegistry</value> -->
    </property>
    <property>
        <name>google.bigtable.project.id</name>
        <value>my-project</value>
    </property>
    <property>
        <name>google.bigtable.instance.id</name>
        <value>my-instance</value>
    </property>
    <property>
        <name>google.bigtable.app_profile.id</name>
        <value>my-app-profile</value>
    </property>
</configuration>
AsyncConnection asyncConnection = ConnectionFactory.createAsyncConnection().get();

…#4043)

* feat: use concrete classes for hbase.client.registry.impl

Previously we used runtime bytecode generation which prevented endusers from using hbase-site.xml to target bigtable in hbase 2 applications

Change-Id: I28c28683419ac8f65bccfa770ab7f0e1cc1a4d2f

* address feedback

Change-Id: Ia58e2cbcbafa57e08a7f710dd5a5b4d3a4e4bf68

* add missing exclue

Change-Id: I452043b55373c1d41024893f5a9781853c9edae7

* wip

Change-Id: I03ab0a4283d430de45417807309498a976e32932

* fix bad rebase

Change-Id: I9de88c819e110e521bdef638d40c63df36ac605b
@igorbernstein2 igorbernstein2 requested a review from a team as a code owner October 16, 2023 14:31
@product-auto-label product-auto-label bot added size: xl Pull request size is extra large. api: bigtable Issues related to the googleapis/java-bigtable-hbase API. labels Oct 16, 2023
@igorbernstein2 igorbernstein2 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 16, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 16, 2023
Change-Id: If99eab1f51a0749fda495d656193f06d9bf1be01
Change-Id: I58cb63d04697537085aeca41d3614e421a5f96ef
Change-Id: I2996531cb702135d14bfebab056ae335bd126c25
Change-Id: I70e9eab35662dfb2faf15a383a44b65668f6bdd5
@igorbernstein2 igorbernstein2 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 16, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 16, 2023
Copy link
Member

@rkaregar rkaregar left a comment

Choose a reason for hiding this comment

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

Do we need any new tests to make sure everything works (e.g., passing the new BigtableConnectionRegistry class or using Async APIs) or they are already covered by the existing tests?

@igorbernstein2
Copy link
Collaborator Author

I plan on adding cross version tests, but that will be a much bigger change. I will add them soon

@igorbernstein2 igorbernstein2 added the owlbot:run Add this label to trigger the Owlbot post processor. label Oct 18, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Oct 18, 2023
@igorbernstein2 igorbernstein2 added the automerge Merge the pull request once unit tests and other checks pass. label Oct 18, 2023
@gcf-merge-on-green gcf-merge-on-green bot merged commit cc22f2f into googleapis:main Oct 18, 2023
12 of 13 checks passed
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigtable Issues related to the googleapis/java-bigtable-hbase API. size: xl Pull request size is extra large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants