Skip to content

Fixing RestIntegTestTask runtime configuration#1

Merged
dblock merged 1 commit intodblock:upgrade-1.2.1from
reta:upgrade-1.2.1
Dec 23, 2021
Merged

Fixing RestIntegTestTask runtime configuration#1
dblock merged 1 commit intodblock:upgrade-1.2.1from
reta:upgrade-1.2.1

Conversation

@reta
Copy link
Copy Markdown

@reta reta commented Dec 23, 2021

Signed-off-by: Andriy Redko andriy.redko@aiven.io

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
}

tasks.withType(RestIntegTestTask)*.configure {
classpath += files(project.configurations.runtimeClasspath.findAll { it.name.contains("log4j-core") })
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Just to explain what I found out:

  • the RestIntegTestTask needs log4j-core (it is used by test scaffolding)
  • in OpenSearch core, log4j-core comes as part of server module but this dependency is optional
  • it seems like when dependencies are declared within the same multimodule build using project, the optional dependencies are included, otherwise - they are not

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

PS: For some reasons, just adding the log4j-core to dependencies section does not solve the problem, may be because RestIntegTestPlugin includes only org.opensearch:opensearch module dependencies? I did not fully understand that yet ...

Copy link
Copy Markdown

@AmiStrn AmiStrn Dec 23, 2021

Choose a reason for hiding this comment

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

Great find. This is tricky
The way these dependencies are set is a pain point for plugin developers.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yeah, it took me a while to dig through classloaders & co to find out that :-(

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Daaam.... @reta Can you please open a bug in OpenSearch describing this mess and maybe suggesting how we can get out of it?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

👍 will do that

@dblock
Copy link
Copy Markdown
Owner

dblock commented Dec 23, 2021

MUCH thanks! I couldn't figure this one out.

@dblock dblock merged commit 94a5316 into dblock:upgrade-1.2.1 Dec 23, 2021
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.

3 participants