Skip to content
This repository has been archived by the owner on Sep 26, 2023. It is now read-only.

feat: relocate Netty Native Image configurations from java-core to gax #1638

Merged
merged 13 commits into from
Mar 31, 2022

Conversation

mpeddada1
Copy link
Contributor

This PR moves the Netty Native Image configurations from java-core to gax. Tested this out manually with Pub/Sub(includes gax+ gax-grpc as a dependency), Bigquery(includes only gax as a dependency) and Secretmanager(generated library). This PR is paired with googleapis/java-core#771

@mpeddada1 mpeddada1 changed the title feat(java):relocate Netty Native Image configurations from java-core to gax chore: relocate Netty Native Image configurations from java-core to gax Mar 25, 2022
@mpeddada1 mpeddada1 requested review from suztomo and meltsufin March 25, 2022 14:26
import org.graalvm.nativeimage.hosted.RuntimeReflection;

/** Internal class offering helper methods for registering methods/classes for reflection. */
public class NativeImageUtils {
Copy link
Member

Choose a reason for hiding this comment

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

Add annotation to indicate this is an internal API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, added the internal API annotation.

}

/** Registers all the classes under the specified package for reflection. */
public static void registerPackageForReflection(FeatureAccess access, String packageName) {
Copy link
Member

Choose a reason for hiding this comment

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

Let's remove this as it's not used in gax.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, removed this method.

* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/

package com.google.nativeimage;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To verify once again, @meltsufin @suztomo is this an acceptable package name? It looks like we have some classes under package com.google.api.gax.grpc and some under com.google in gax-grpc.

Copy link
Member

Choose a reason for hiding this comment

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

I think it should actually go into "com.google.api.gax.grpc.nativeimage".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah okay, corrected this. Thank you!

@sonarcloud
Copy link

sonarcloud bot commented Mar 31, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@mpeddada1 mpeddada1 changed the title chore: relocate Netty Native Image configurations from java-core to gax feat: relocate Netty Native Image configurations from java-core to gax Mar 31, 2022
@mpeddada1 mpeddada1 merged commit aafded4 into main Mar 31, 2022
@mpeddada1 mpeddada1 deleted the relocate-netty branch March 31, 2022 18:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants