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.

Sign up for GitHub

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 x-pack core pull transport-nio #32757

Merged
merged 7 commits into from
Aug 10, 2018
Merged

Make x-pack core pull transport-nio #32757

Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
825537f
Make x-pack core pull transport-nio
Tim-Brooks Aug 9, 2018
b9b3f60
Add comment
Tim-Brooks Aug 9, 2018
d2f54c6
Merge remote-tracking branch 'upstream/master' into fix_x_pack_with_nio
Tim-Brooks Aug 10, 2018
dc62e9d
Make changes
Tim-Brooks Aug 10, 2018
b2d0b53
Whitespace
Tim-Brooks Aug 10, 2018
94b29b4
Changes
Tim-Brooks Aug 10, 2018
803b11c
Take ryans comment
Tim-Brooks Aug 10, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions x-pack/plugin/core/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,17 @@ dependencies {
// security deps
shadow 'com.unboundid:unboundid-ldapsdk:3.2.0'
shadow project(path: ':modules:transport-netty4', configuration: 'runtime')
shadow(project(path: ':plugins:transport-nio', configuration: 'runtime')) {
// TODO: This should not be necessary if core is compile only
Copy link
Member

Choose a reason for hiding this comment

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

I would expand this comment a bit. Something like:

// TODO: core exclusion should not be necessary, since it is a transitive dep of all plugins

Also, I wonder if this is due to how we add server as a compile dep, but this is shadow here? I wonder if @nik9000's upcoming changes to add bundle will make this better?

exclude group: "org.elasticsearch", module: "elasticsearch-core"
Copy link
Member

Choose a reason for hiding this comment

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

This looks like a bug in esplugin, that core is not being marked as compileOnly in this case (yet it is for depending on the transport-netty4 module). @atorok can you investigate this and open an issue? @tbrooks8 can you add a TODO/comment here that this should not be necessary

exclude group: "io.netty", module: "netty-buffer"
Copy link
Contributor

Choose a reason for hiding this comment

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

We disable transitive dependencies for anything but org.elasticsearch so these excludes shouldn't be required either.

Copy link
Member

@rjernst rjernst Aug 10, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

transport-nio is under org.elasticsearch, that is why it gets transitive deps.

exclude group: "io.netty", module: "netty-codec"
exclude group: "io.netty", module: "netty-codec-http"
exclude group: "io.netty", module: "netty-common"
exclude group: "io.netty", module: "netty-handler"
exclude group: "io.netty", module: "netty-resolver"
exclude group: "io.netty", module: "netty-transport"
}

testCompile 'org.elasticsearch:securemock:1.2'
testCompile "org.elasticsearch:mocksocket:${versions.mocksocket}"
Expand Down