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

Lays the groundwork for permitting other Netty transports while retaining Netty as an implementation-only concern. #2478

Merged
merged 7 commits into from
Jan 6, 2021

Conversation

ljnelson
Copy link
Member

Signed-off-by: Laird Nelson laird.nelson@oracle.com

@ljnelson ljnelson self-assigned this Oct 22, 2020
@ljnelson
Copy link
Member Author

ljnelson commented Oct 22, 2020

Constraints in play:

  • WebServer is a public interface that defines its own Builder class which as of this writing creates a NettyWebServer.
  • ServerConfiguration is a public interface that is WebServer-implementation agnostic.
  • NettyWebServer is not public.
  • Custom Netty transports require extra dependencies that are not suitable on all architectures.
  • A Netty transport is not a single thing, but a loose agglomeration of other things. All native Netty transports feature a single utility class that has an isAvailable() method. See, for example, Epoll.isAvailable(), which determines if the architecture is suitable for that combination of transport and its native libraries. It is expected that if the transport is not available, then whoever is putting Netty together will use a different one.
  • Some transports are always available and so do not feature this check. For example, the NIO transport does not have a utility class and is available on modern JVMs.

@ljnelson
Copy link
Member Author

Related issue: #2444; this PR does not directly address this issue but lays the groundwork for a subsequent PR to deal with it.

@ljnelson ljnelson added this to In Progress in Backlog Oct 22, 2020
@ljnelson ljnelson added 2.x Issues for 2.x version branch enhancement New feature or request P4 labels Oct 22, 2020
@m0mus m0mus added P3 and removed P4 labels Dec 17, 2020
@ljnelson ljnelson marked this pull request as ready for review December 17, 2020 18:08
…ning Netty as an implementation-only concern.

Signed-off-by: Laird Nelson <laird.nelson@oracle.com>
Signed-off-by: Laird Nelson <laird.nelson@oracle.com>
…and installation of Transport

Signed-off-by: Laird Nelson <laird.nelson@oracle.com>
Signed-off-by: Laird Nelson <laird.nelson@oracle.com>
…encies/pom.xml to go back to 2020

Signed-off-by: Laird Nelson <laird.nelson@oracle.com>
…pyright to be 2021

Signed-off-by: Laird Nelson <laird.nelson@oracle.com>
…name is clearer

Signed-off-by: Laird Nelson <laird.nelson@oracle.com>
Comment on lines +344 to +361
<dependency>
<groupId>io.netty</groupId>
<artifactId>netty-transport-native-epoll</artifactId>
<version>${version.lib.netty}</version>
<classifier>linux-x86_64</classifier>
</dependency>
<dependency>
<groupId>io.netty</groupId>
<artifactId>netty-transport-native-epoll</artifactId>
<version>${version.lib.netty}</version>
<classifier>linux-aarch64</classifier>
</dependency>
<dependency>
<groupId>io.netty</groupId>
<artifactId>netty-transport-native-kqueue</artifactId>
<version>${version.lib.netty}</version>
<classifier>osx-x86_64</classifier>
</dependency>
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these actually required by this PR or just in a followup? If the latter, maybe best to leave them out for now.

Copy link
Member Author

Choose a reason for hiding this comment

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

These are just the <dependencyManagement> entries (not <dependencies> entries). They do no harm going in now IMHO. A subsequent PR will actually declare dependencies that can use them.

@ljnelson ljnelson merged commit c888eca into helidon-io:master Jan 6, 2021
Backlog automation moved this from In Progress to Closed Jan 6, 2021
@ljnelson ljnelson deleted the netty-epoll branch January 6, 2021 02:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.x Issues for 2.x version branch enhancement New feature or request P3
Projects
Backlog
  
Closed
Development

Successfully merging this pull request may close these issues.

None yet

5 participants