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

Make initializer as static #13506

Merged
merged 3 commits into from
Jul 24, 2023
Merged

Conversation

hyperxpro
Copy link
Contributor

Motivation:
I went through native transport and found that we don't explicitly mark class initializers as static. We should mark them as static.

Modification:
Made initializers as static

Result:
Clean code and no more IDE warning pings.

@hyperxpro
Copy link
Contributor Author

2023-07-22T10:51:32.8068338Z [ERROR] Failed to execute goal on project netty-transport-native-epoll: Could not resolve dependencies for project io.netty:netty-transport-native-epoll:jar:4.1.96.Final-SNAPSHOT: Failed to collect dependencies at io.github.artsok:rerunner-jupiter:jar:2.1.6: Failed to read artifact descriptor for io.github.artsok:rerunner-jupiter:jar:2.1.6: The following artifacts could not be resolved: io.github.artsok:rerunner-jupiter:pom:2.1.6 (absent): Could not transfer artifact io.github.artsok:rerunner-jupiter:pom:2.1.6 from/to central (https://repo.maven.apache.org/maven2): Connection reset -> [Help 1]

@normanmaurer normanmaurer merged commit 1c7f0fa into netty:4.1 Jul 24, 2023
14 checks passed
@normanmaurer normanmaurer added this to the 4.1.96.Final milestone Jul 24, 2023
@normanmaurer
Copy link
Member

Thanks for the cleanup

@Kavindu-Dodan
Copy link

Unfortunately, this breaks non linux usage of the given classes. At least I experienced this with EpollEventLoopGroup which it is not compatible with non linunx systems. The root cause is the execution of static block at class loading time. Previously, this (Epoll.ensureAvailability()) is skipped and only got executed when initializing the class

@chrisvest
Copy link
Contributor

@Kavindu-Dodan How are you "using" this class on non-Linux systems?

The exception is not thrown if you do something like the following, for instance:

EventLoopGroup group = Epoll.isAvailable() ? new EpollEventLoopGroup(1) : new NioEventLoopGroup(1);

@Kavindu-Dodan
Copy link

Kavindu-Dodan commented Aug 3, 2023

@chrisvest I observed this with one of the tests where we use Mockito.mockConstruction 1. Internally, this performs a class loading, hence the errors.

Yes, the proposed solution works if I have control over the loigc. However, it might not be possible for other usages.

Footnotes

  1. https://github.com/open-feature/java-sdk-contrib/blob/dev.openfeature.contrib.providers.flagd-v0.6.0/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdProviderTest.java#L106-L119

normanmaurer added a commit that referenced this pull request Aug 16, 2023
Motivations:

By changing the code to use a static block we did remove the ability to load the class at all. This is sort of a regression as it may break existing code.

Modifications:

This reverts commit 1c7f0fa.

Result:

It's possible again to at least load the class all the time as before.
Fixes #13523
normanmaurer added a commit that referenced this pull request Aug 17, 2023
Motivations:

By changing the code to use a static block we did remove the ability to
load the class at all. This is sort of a regression as it may break
existing code.

Modifications:

This reverts commit 1c7f0fa.

Result:

It's possible again to at least load the class all the time as before.
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