You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "paul-rogers (via GitHub)" <gi...@apache.org> on 2023/03/02 21:47:00 UTC
[GitHub] [druid] paul-rogers opened a new pull request, #13877: Extend the IT framework to allow tests in extensions
paul-rogers opened a new pull request, #13877:
URL: https://github.com/apache/druid/pull/13877
The "new" IT framework provides a convenient way to package and run integration tests (ITs), but only for core modules. We have a use case to run an IT for a `contrib` extension: the proposed gRPC query extension. This PR provides the IT framework functionality to allow non-core ITs.
The `it.sh` script takes an option third parameter to specify the relative path to the Maven module that holds the test:
```bash
./it.sh test MyCategory extensions-contrib/my-extension-it
```
Because of the Maven build order, the ITs for an extension must be in a Maven module separate from the extension itself. In this case, `my-extension` is the extension, while `my-extension-it` are the ITs for that extension.
The layout in the extension mimics the existing `cases` module. Provide a Maven file that runs the tests using FailSafe. Provide a `cluster` directory structured as explained in the IT docs.
#### Optional Extensions Path
To test an extension, we first have to load it onto the IT cluster. One way to do that would be to add it to the standard IT test image. However, that's an awkward choice for a community extension. Another choice would be to build a separate image for each community extension, which would also be awkward.
The solution provided here is to add an extensions search path to Druid configuration. If `druid.extensions.path` is set, it names additional directories to search for extensions. Example:
```text
druid.extensions.path=["/shared/extensions"]
```
The `path` is empty by default. If present, the "effective path" is a list consisting of first the `directory` setting, followed by the `path` items.
This feature is used in an IT to add the contrib extension to an `extensions` folder in the "shared" directory before starting the IT cluster. The setup work is done in the `setup.sh` script (see below) just before cluster start. The `path` allows us to specify an effective path of `$DRUID_HOME/extensions,/shared/extensions`. With this, the contrib extension is found as normal, if the extension is listed in `loadList`.
The `launch.sh` script in the IT container looks for `/shared/extensions`. If found, it does the work to define the `path` property. Tests _do not_ need to add this property to the (generated) `docker-compose.yaml` file.
This change makes `directory` optional. However, it defaults to `extensions`. For completeness, this PR allows a value of a blank string, which means "ignore `directory`, just use `path`."
The new `path` variable is completely transparent to the user. Current behavior does not change. The only users who might need this feature are those running under K8s who would benefit from providing extensions as file mounts.
#### Revision to IT Structure
As part of this work, several changes were made to the structure of the existing IT directory.
* The Python "template" for Docker Compose moves from `templates/Category.py` to `cluster/Category/docker-compose.py`.
* A new 'cluster/Category/verify.sh` script peforms pre-test checks. The awkward code that was in `.it.sh` to check for required environment variables moved into `verify.sh`.
* A new `cluster/Category/setup.sh` script does any additional setup needed before launching an IT cluster. Use this to add additional files to the `shared` directory, for example. Using the script is _far_ easier than trying to script in Maven.
* The cluster startup and shutdown steps were removed from `cases/pom.xml`. We were not using them: we now use `it.sh` to do this work.
* The `it.sh` and `cluster.sh` scripts are now aware of two modules: the "base" `cases` module, and the "test" module. Before now, those were the same. Not they can differ.
#### Prevent Snapshot Downloading
Also added configuration to the root `pom.xml` file to prevent an attempt to download snapshots from upstream. This most often happens when dependencies are missing and Maven cannot find a required Druid module, and instead tries to ask upstream repos. Bad Things would happen if it actually found such jars.
#### Release note
Added the `druid.extensions.path` config setting. This setting is optional and is normally not set. Primarily for testing. See `configuration/index.md` for details.
<hr>
This PR has:
- [X] been self-reviewed.
- [X] added internal documentation for new or modified features or behaviors.
- [X] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
- [X] added integration tests.
- [X] been tested in an integration test Druid cluster.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] paul-rogers commented on pull request #13877: Extend the IT framework to allow tests in extensions
Posted by "paul-rogers (via GitHub)" <gi...@apache.org>.
paul-rogers commented on PR #13877:
URL: https://github.com/apache/druid/pull/13877#issuecomment-1496494950
The Kafka tests have failed for the last month. At first I thought it was because the PR contained a change to the way Druid locates extensions. But, I removed that code (having found another solution to adding test-specific extensions), yet Kafka still fails. I suspect the test itself is flaky. The test was converted to the new IT framework to allow debugging (in a separate PR). However, the test ran perfectly on the local machine after the conversion. So, let's just assume the Kafka test is flaky and ignore it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] abhishekagarwal87 merged pull request #13877: Extend the IT framework to allow tests in extensions
Posted by "abhishekagarwal87 (via GitHub)" <gi...@apache.org>.
abhishekagarwal87 merged PR #13877:
URL: https://github.com/apache/druid/pull/13877
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] abhishekrb19 commented on a diff in pull request #13877: Extend the IT framework to allow tests in extensions
Posted by "abhishekrb19 (via GitHub)" <gi...@apache.org>.
abhishekrb19 commented on code in PR #13877:
URL: https://github.com/apache/druid/pull/13877#discussion_r1124936963
##########
integration-tests-ex/cases/cluster.sh:
##########
@@ -27,15 +27,16 @@ set -e
# Enable for debugging
#set -x
-export MODULE_DIR=$(cd $(dirname $0) && pwd)
+export BASE_MODULE_DIR=$(cd $(dirname $0) && pwd)
+
+# The location of the tests, if different than the
Review Comment:
nit: incomplete comment
##########
it.sh:
##########
@@ -41,22 +41,27 @@ Usage: $0 cmd [category]
Build druid-it-tools
image
Build the test image
- up <category>
+ up <category> [<module>]
Start the cluster for category
- down <category>
+ down <category> [<module>]
Stop the cluster for category
- test <category>
+ test <category> [<module>]
Start the cluster, run the test for category, and stop the cluster
- tail <category>
+ tail <category> [<module>]
Show the last 20 lines of each container log
- gen
+ gen <category> [<module>]
Generate docker-compose.yaml files (done automatically on up)
run one IT in Travis (build dist, image, run test, tail logs)
- github <category>
+ github <category> [<module>]
Run one IT in Github Workflows (run test, tail logs)
prune
prune Docker volumes
+Arguments:
+ category: A defined IT JUnit category, and IT-<category> profile
Review Comment:
nice!
##########
integration-tests-ex/cases/pom.xml:
##########
@@ -21,517 +21,477 @@
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
- <modelVersion>4.0.0</modelVersion>
+ <modelVersion>4.0.0</modelVersion>
- <groupId>org.apache.druid.integration-tests</groupId>
- <artifactId>druid-it-cases</artifactId>
- <name>druid-it-cases</name>
- <description>New Integration Tests</description>
+ <groupId>org.apache.druid.integration-tests</groupId>
+ <artifactId>druid-it-cases</artifactId>
+ <name>druid-it-cases</name>
+ <description>New Integration Tests</description>
- <parent>
- <groupId>org.apache.druid</groupId>
- <artifactId>druid</artifactId>
- <version>26.0.0-SNAPSHOT</version>
- <relativePath>../../pom.xml</relativePath>
- </parent>
+ <parent>
+ <groupId>org.apache.druid</groupId>
+ <artifactId>druid</artifactId>
+ <version>26.0.0-SNAPSHOT</version>
+ <relativePath>../../pom.xml</relativePath>
+ </parent>
- <properties>
- <it.category>MustSetACategory</it.category>
- </properties>
+ <properties>
+ <it.category>MustSetACategory</it.category>
+ </properties>
- <dependencies>
- <dependency>
- <groupId>org.apache.druid</groupId>
- <artifactId>druid-integration-tests</artifactId>
- <version>${project.parent.version}</version>
- </dependency>
- <!-- See https://maven.apache.org/plugins/maven-jar-plugin/examples/create-test-jar.html -->
- <dependency>
- <groupId>org.apache.druid</groupId>
- <artifactId>druid-integration-tests</artifactId>
- <version>${project.parent.version}</version>
- <classifier>tests</classifier>
- <type>test-jar</type>
- <scope>test</scope>
- </dependency>
- <dependency>
- <groupId>org.apache.druid</groupId>
- <artifactId>druid-processing</artifactId>
- <version>${project.parent.version}</version>
- </dependency>
- <dependency>
- <groupId>org.apache.druid</groupId>
- <artifactId>druid-server</artifactId>
- <version>${project.parent.version}</version>
- </dependency>
- <dependency>
- <groupId>org.apache.druid</groupId>
- <artifactId>druid-services</artifactId>
- <version>${project.parent.version}</version>
- </dependency>
- <dependency>
- <groupId>org.apache.druid</groupId>
- <artifactId>druid-indexing-service</artifactId>
- <version>${project.parent.version}</version>
- </dependency>
- <dependency>
- <groupId>com.google.inject</groupId>
- <artifactId>guice</artifactId>
- </dependency>
- <dependency>
- <groupId>com.google.inject.extensions</groupId>
- <artifactId>guice-multibindings</artifactId>
- </dependency>
- <dependency>
- <groupId>org.apache.curator</groupId>
- <artifactId>curator-framework</artifactId>
- </dependency>
- <dependency>
- <groupId>com.google.guava</groupId>
- <artifactId>guava</artifactId>
- </dependency>
- <dependency>
- <groupId>com.fasterxml.jackson.dataformat</groupId>
- <artifactId>jackson-dataformat-yaml</artifactId>
- </dependency>
- <dependency>
- <groupId>com.fasterxml.jackson.core</groupId>
- <artifactId>jackson-annotations</artifactId>
- </dependency>
- <dependency>
- <groupId>com.fasterxml.jackson.core</groupId>
- <artifactId>jackson-databind</artifactId>
- </dependency>
- <dependency>
- <groupId>com.fasterxml.jackson.core</groupId>
- <artifactId>jackson-core</artifactId>
- </dependency>
- <dependency>
- <groupId>joda-time</groupId>
- <artifactId>joda-time</artifactId>
- </dependency>
- <dependency>
- <groupId>commons-io</groupId>
- <artifactId>commons-io</artifactId>
- </dependency>
- <dependency>
- <groupId>com.github.docker-java</groupId>
- <artifactId>docker-java-core</artifactId>
- </dependency>
- <dependency>
- <groupId>com.github.docker-java</groupId>
- <artifactId>docker-java-api</artifactId>
- </dependency>
- <dependency>
- <groupId>com.github.docker-java</groupId>
- <artifactId>docker-java-transport-netty</artifactId>
- </dependency>
- <dependency>
- <groupId>com.github.docker-java</groupId>
- <artifactId>docker-java</artifactId>
- <exclusions>
- <exclusion>
- <groupId>com.github.docker-java</groupId>
- <artifactId>docker-java-transport-jersey</artifactId>
- </exclusion>
- <exclusion>
- <groupId>io.netty</groupId>
- <artifactId>netty-transport-native-kqueue</artifactId>
- </exclusion>
- </exclusions>
- </dependency>
- <dependency>
- <groupId>com.amazonaws</groupId>
- <artifactId>aws-java-sdk-core</artifactId>
- <version>${aws.sdk.version}</version>
- </dependency>
- <dependency>
- <groupId>com.google.api-client</groupId>
- <artifactId>google-api-client</artifactId>
- <version>${com.google.apis.client.version}</version>
- <scope>provided</scope>
- </dependency>
- <dependency>
- <groupId>com.google.apis</groupId>
- <artifactId>google-api-services-storage</artifactId>
- <version>${com.google.apis.storage.version}</version>
- <exclusions>
- <exclusion>
- <groupId>com.google.api-client</groupId>
- <artifactId>google-api-client</artifactId>
- </exclusion>
- </exclusions>
- </dependency>
- <dependency>
- <groupId>com.microsoft.azure</groupId>
- <artifactId>azure-storage</artifactId>
- <version>8.6.0</version>
- <exclusions>
- <exclusion>
- <groupId>org.slf4j</groupId>
- <artifactId>slf4j-api</artifactId>
- </exclusion>
- <exclusion>
- <groupId>com.fasterxml.jackson.core</groupId>
- <artifactId>jackson-core</artifactId>
- </exclusion>
- <exclusion>
- <groupId>org.apache.commons</groupId>
- <artifactId>commons-lang3</artifactId>
- </exclusion>
- <exclusion>
- <groupId>com.google.guava</groupId>
- <artifactId>guava</artifactId>
- </exclusion>
- </exclusions>
- </dependency>
- <dependency>
- <groupId>com.google.http-client</groupId>
- <artifactId>google-http-client</artifactId>
- <version>${com.google.apis.client.version}</version>
- <scope>provided</scope>
- </dependency>
- <dependency>
- <groupId>com.google.http-client</groupId>
- <artifactId>google-http-client-jackson2</artifactId>
- <version>${com.google.apis.client.version}</version>
- <scope>provided</scope>
- </dependency>
- <!-- Tests can choose either the MySQL or MariaDB driver. -->
- <dependency>
- <groupId>mysql</groupId>
- <artifactId>mysql-connector-java</artifactId>
- <version>${mysql.version}</version>
- <scope>runtime</scope>
- </dependency>
- <dependency>
- <groupId>org.mariadb.jdbc</groupId>
- <artifactId>mariadb-java-client</artifactId>
- <version>${mariadb.version}</version>
- <scope>runtime</scope>
- </dependency>
- <dependency>
- <groupId>org.jdbi</groupId>
- <artifactId>jdbi</artifactId>
- </dependency>
- <dependency>
- <groupId>org.apache.druid.extensions</groupId>
- <artifactId>mysql-metadata-storage</artifactId>
- <version>${project.parent.version}</version>
- </dependency>
- <dependency>
- <groupId>org.apache.druid.extensions</groupId>
- <artifactId>druid-azure-extensions</artifactId>
- <version>${project.parent.version}</version>
- <scope>provided</scope>
- </dependency>
- <dependency>
- <groupId>com.amazonaws</groupId>
- <artifactId>aws-java-sdk-s3</artifactId>
- <version>${aws.sdk.version}</version>
- <scope>provided</scope>
- </dependency>
- <dependency>
- <groupId>org.apache.druid.extensions</groupId>
- <artifactId>druid-s3-extensions</artifactId>
- <version>${project.parent.version}</version>
- <scope>provided</scope>
- </dependency>
- <dependency>
- <groupId>org.apache.druid.extensions</groupId>
- <artifactId>druid-hdfs-storage</artifactId>
- <version>${project.parent.version}</version>
- <scope>provided</scope>
- <exclusions>
- <exclusion>
- <groupId>com.amazonaws</groupId>
- <artifactId>aws-java-sdk-bundle</artifactId>
- </exclusion>
- </exclusions>
- </dependency>
- <dependency>
- <groupId>org.apache.druid.extensions</groupId>
- <artifactId>druid-google-extensions</artifactId>
- <version>${project.parent.version}</version>
- <scope>provided</scope>
- </dependency>
- <dependency>
- <groupId>org.apache.druid</groupId>
- <artifactId>druid-gcp-common</artifactId>
- <version>${project.parent.version}</version>
- <scope>provided</scope>
- </dependency>
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.druid</groupId>
+ <artifactId>druid-integration-tests</artifactId>
+ <version>${project.parent.version}</version>
+ </dependency>
+ <!-- See https://maven.apache.org/plugins/maven-jar-plugin/examples/create-test-jar.html -->
+ <dependency>
+ <groupId>org.apache.druid</groupId>
+ <artifactId>druid-integration-tests</artifactId>
+ <version>${project.parent.version}</version>
+ <classifier>tests</classifier>
+ <type>test-jar</type>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.druid</groupId>
+ <artifactId>druid-processing</artifactId>
+ <version>${project.parent.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.druid</groupId>
+ <artifactId>druid-server</artifactId>
+ <version>${project.parent.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.druid</groupId>
+ <artifactId>druid-services</artifactId>
+ <version>${project.parent.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.druid</groupId>
+ <artifactId>druid-indexing-service</artifactId>
+ <version>${project.parent.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>com.google.inject</groupId>
+ <artifactId>guice</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.google.inject.extensions</groupId>
+ <artifactId>guice-multibindings</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.curator</groupId>
+ <artifactId>curator-framework</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.google.guava</groupId>
+ <artifactId>guava</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.fasterxml.jackson.dataformat</groupId>
+ <artifactId>jackson-dataformat-yaml</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.fasterxml.jackson.core</groupId>
+ <artifactId>jackson-annotations</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.fasterxml.jackson.core</groupId>
+ <artifactId>jackson-databind</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.fasterxml.jackson.core</groupId>
+ <artifactId>jackson-core</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>joda-time</groupId>
+ <artifactId>joda-time</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>commons-io</groupId>
+ <artifactId>commons-io</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.github.docker-java</groupId>
+ <artifactId>docker-java-core</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.github.docker-java</groupId>
+ <artifactId>docker-java-api</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.github.docker-java</groupId>
+ <artifactId>docker-java-transport-netty</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.github.docker-java</groupId>
+ <artifactId>docker-java</artifactId>
+ <exclusions>
+ <exclusion>
+ <groupId>com.github.docker-java</groupId>
+ <artifactId>docker-java-transport-jersey</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>io.netty</groupId>
+ <artifactId>netty-transport-native-kqueue</artifactId>
+ </exclusion>
+ </exclusions>
+ </dependency>
+ <dependency>
+ <groupId>com.amazonaws</groupId>
+ <artifactId>aws-java-sdk-core</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.google.api-client</groupId>
+ <artifactId>google-api-client</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.google.apis</groupId>
+ <artifactId>google-api-services-storage</artifactId>
+ <version>${com.google.apis.storage.version}</version>
+ <exclusions>
+ <exclusion>
+ <groupId>com.google.api-client</groupId>
+ <artifactId>google-api-client</artifactId>
+ </exclusion>
+ </exclusions>
+ </dependency>
+ <dependency>
+ <groupId>com.microsoft.azure</groupId>
+ <artifactId>azure-storage</artifactId>
+ <version>8.6.0</version>
+ <exclusions>
+ <exclusion>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-api</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>com.fasterxml.jackson.core</groupId>
+ <artifactId>jackson-core</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.commons</groupId>
+ <artifactId>commons-lang3</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>com.google.guava</groupId>
+ <artifactId>guava</artifactId>
+ </exclusion>
+ </exclusions>
+ </dependency>
+ <dependency>
+ <groupId>com.google.http-client</groupId>
+ <artifactId>google-http-client</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.google.http-client</groupId>
+ <artifactId>google-http-client-jackson2</artifactId>
+ </dependency>
+ <!-- Tests can choose either the MySQL or MariaDB driver. -->
+ <dependency>
+ <groupId>mysql</groupId>
+ <artifactId>mysql-connector-java</artifactId>
+ <version>${mysql.version}</version>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.mariadb.jdbc</groupId>
+ <artifactId>mariadb-java-client</artifactId>
+ <version>${mariadb.version}</version>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.jdbi</groupId>
+ <artifactId>jdbi</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.druid.extensions</groupId>
+ <artifactId>mysql-metadata-storage</artifactId>
+ <version>${project.parent.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.druid.extensions</groupId>
+ <artifactId>druid-azure-extensions</artifactId>
+ <version>${project.parent.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>com.amazonaws</groupId>
+ <artifactId>aws-java-sdk-s3</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.druid.extensions</groupId>
+ <artifactId>druid-s3-extensions</artifactId>
+ <version>${project.parent.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.druid.extensions</groupId>
+ <artifactId>druid-hdfs-storage</artifactId>
+ <version>${project.parent.version}</version>
+ <scope>provided</scope>
+ <exclusions>
+ <exclusion>
+ <groupId>com.amazonaws</groupId>
+ <artifactId>aws-java-sdk-bundle</artifactId>
+ </exclusion>
+ </exclusions>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.druid.extensions</groupId>
+ <artifactId>druid-google-extensions</artifactId>
+ <version>${project.parent.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.druid</groupId>
+ <artifactId>druid-gcp-common</artifactId>
+ <version>${project.parent.version}</version>
+ </dependency>
- <dependency>
- <groupId>org.apache.druid</groupId>
- <artifactId>druid-sql</artifactId>
- <version>${project.parent.version}</version>
- </dependency>
- <dependency>
- <groupId>org.apache.druid.extensions</groupId>
- <artifactId>druid-multi-stage-query</artifactId>
- <version>${project.parent.version}</version>
- <scope>provided</scope>
- </dependency>
- <dependency>
- <groupId>org.apache.druid.extensions</groupId>
- <artifactId>druid-catalog</artifactId>
- <version>${project.parent.version}</version>
- <scope>provided</scope>
- </dependency>
- <dependency>
- <groupId>org.apache.commons</groupId>
- <artifactId>commons-lang3</artifactId>
- </dependency>
- <dependency>
- <groupId>io.netty</groupId>
- <artifactId>netty</artifactId>
- </dependency>
- <dependency>
- <groupId>org.apache.kafka</groupId>
- <artifactId>kafka-clients</artifactId>
- <version>${apache.kafka.version}</version>
- <scope>test</scope>
- </dependency>
- <dependency>
- <groupId>com.google.code.findbugs</groupId>
- <artifactId>jsr305</artifactId>
- </dependency>
- <dependency>
- <groupId>junit</groupId>
- <artifactId>junit</artifactId>
- <scope>test</scope>
- </dependency>
- <dependency>
- <groupId>pl.pragmatists</groupId>
- <artifactId>JUnitParams</artifactId>
- <scope>test</scope>
- </dependency>
- <dependency>
- <groupId>javax.ws.rs</groupId>
- <artifactId>jsr311-api</artifactId>
- </dependency>
- <dependency>
- <groupId>org.apache.curator</groupId>
- <artifactId>curator-client</artifactId>
- <version>5.4.0</version>
- </dependency>
- </dependencies>
+ <dependency>
+ <groupId>org.apache.druid</groupId>
+ <artifactId>druid-sql</artifactId>
+ <version>${project.parent.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.druid.extensions</groupId>
+ <artifactId>druid-multi-stage-query</artifactId>
+ <version>${project.parent.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.druid.extensions</groupId>
+ <artifactId>druid-catalog</artifactId>
+ <version>${project.parent.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.commons</groupId>
+ <artifactId>commons-lang3</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>io.netty</groupId>
+ <artifactId>netty</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.kafka</groupId>
+ <artifactId>kafka-clients</artifactId>
+ <version>${apache.kafka.version}</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>com.google.code.findbugs</groupId>
+ <artifactId>jsr305</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>junit</groupId>
+ <artifactId>junit</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>pl.pragmatists</groupId>
+ <artifactId>JUnitParams</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>javax.ws.rs</groupId>
+ <artifactId>jsr311-api</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.curator</groupId>
+ <artifactId>curator-client</artifactId>
+ </dependency>
+ </dependencies>
- <!-- Exclude ITs from surefire. Required because they end with "Test". -->
- <build>
+ <!-- Exclude ITs from surefire. Required because they end with "Test". -->
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <configuration>
+ <excludes>
+ <exclude>**/IT*</exclude>
+ </excludes>
+ </configuration>
+ </plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-dependency-plugin</artifactId>
+ <configuration>
+ <!--
+ ~ The analysis gets confused between javax.xml.bind:jaxb-api and jakarta.xml.bind:jakarta.xml.bind-api.
+ ~ The latter is a transitive dependency of jackson 2.10+.
+ -->
+ <ignoredUsedUndeclaredDependencies>
+ <ignoredUsedUndeclaredDependency>org.glassfish.hk2.external:jakarta.inject</ignoredUsedUndeclaredDependency>
+ </ignoredUsedUndeclaredDependencies>
+ <!-- Dynamically loaded. -->
+ <ignoredUnusedDeclaredDependencies>
+ <ignoredUnusedDeclaredDependency>mysql:mysql-connector-java:jar</ignoredUnusedDeclaredDependency>
+ </ignoredUnusedDeclaredDependencies>
+ </configuration>
+ </plugin>
+ <plugin>
+ <groupId>org.owasp</groupId>
+ <artifactId>dependency-check-maven</artifactId>
+ <configuration>
+ <skip>true</skip>
+ </configuration>
+ </plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-jar-plugin</artifactId>
+ <executions>
+ <execution>
+ <goals>
+ <goal>test-jar</goal>
+ </goals>
+ </execution>
+ </executions>
+ </plugin>
+ </plugins>
+ </build>
+
+ <profiles>
+ <profile>
+ <id>IT-HighAvailability</id>
+ <activation>
+ <activeByDefault>false</activeByDefault>
+ </activation>
+ <properties>
+ <it.category>HighAvailability</it.category>
+ </properties>
+ </profile>
+ <profile>
+ <id>IT-BatchIndex</id>
+ <activation>
+ <activeByDefault>false</activeByDefault>
+ </activation>
+ <properties>
+ <it.category>BatchIndex</it.category>
+ </properties>
+ </profile>
+ <profile>
+ <id>IT-InputSource</id>
+ <activation>
+ <activeByDefault>false</activeByDefault>
+ </activation>
+ <properties>
+ <it.category>InputSource</it.category>
+ </properties>
+ </profile>
+ <profile>
+ <id>IT-InputFormat</id>
+ <activation>
+ <activeByDefault>false</activeByDefault>
+ </activation>
+ <properties>
+ <it.category>InputFormat</it.category>
+ </properties>
+ </profile>
+ <profile>
+ <id>IT-AzureDeepStorage</id>
+ <activation>
+ <activeByDefault>false</activeByDefault>
+ </activation>
+ <properties>
+ <it.category>AzureDeepStorage</it.category>
+ </properties>
+ </profile>
+ <profile>
+ <id>IT-MultiStageQuery</id>
+ <activation>
+ <activeByDefault>false</activeByDefault>
+ </activation>
+ <properties>
+ <it.category>MultiStageQuery</it.category>
+ </properties>
+ </profile>
+ <profile>
+ <id>IT-MultiStageQueryWithMM</id>
+ <activation>
+ <activeByDefault>false</activeByDefault>
+ </activation>
+ <properties>
+ <it.category>MultiStageQueryWithMM</it.category>
+ </properties>
+ </profile>
+ <profile>
+ <id>IT-Catalog</id>
+ <activation>
+ <activeByDefault>false</activeByDefault>
+ </activation>
+ <properties>
+ <it.category>Catalog</it.category>
+ </properties>
+ </profile>
+ <profile>
+ <id>IT-S3DeepStorage</id>
+ <activation>
+ <activeByDefault>false</activeByDefault>
+ </activation>
+ <properties>
+ <it.category>S3DeepStorage</it.category>
+ </properties>
+ </profile>
+ <profile>
+ <id>IT-GcsDeepStorage</id>
+ <activation>
+ <activeByDefault>false</activeByDefault>
+ </activation>
+ <properties>
+ <it.category>GcsDeepStorage</it.category>
+ </properties>
+ </profile>
+ <profile>
+ <!-- Run the integration tests in Docker. Assumes that the cluster has been
+ started via $DRUID_DEV/it.sh up <Category> or $DRUID_DEV/it.sh test <Category>
+ Cluster start/stop is not done here to avoid duplicating functionality.
+ Builds should invoke ITs using it.sh, not directy using Maven.
+ -->
+ <id>docker-tests</id>
+ <activation>
+ <activeByDefault>false</activeByDefault>
+ </activation>
+ <build>
<plugins>
- <plugin>
- <groupId>org.apache.maven.plugins</groupId>
- <artifactId>maven-surefire-plugin</artifactId>
- <configuration>
- <excludes>
- <exclude>**/IT*</exclude>
- </excludes>
- </configuration>
- </plugin>
- <plugin>
- <groupId>org.apache.maven.plugins</groupId>
- <artifactId>maven-dependency-plugin</artifactId>
- <configuration>
- <!--
- ~ The analysis gets confused between javax.xml.bind:jaxb-api and jakarta.xml.bind:jakarta.xml.bind-api.
- ~ The latter is a transitive dependency of jackson 2.10+.
- -->
- <ignoredUsedUndeclaredDependencies>
- <ignoredUsedUndeclaredDependency>org.glassfish.hk2.external:jakarta.inject</ignoredUsedUndeclaredDependency>
- </ignoredUsedUndeclaredDependencies>
- <!-- Dynamically loaded. -->
- <ignoredUnusedDeclaredDependencies>
- <ignoredUnusedDeclaredDependency>mysql:mysql-connector-java:jar</ignoredUnusedDeclaredDependency>
- </ignoredUnusedDeclaredDependencies>
- </configuration>
- </plugin>
- <plugin>
- <groupId>org.owasp</groupId>
- <artifactId>dependency-check-maven</artifactId>
+ <plugin>
+ <artifactId>maven-failsafe-plugin</artifactId>
+ <version>3.0.0-M7</version>
+ <dependencies>
+ <!-- Required to force Failsafe to use JUnit instead of TestNG.
+ junit47 is required to use test categories. -->
+ <dependency>
+ <groupId>org.apache.maven.surefire</groupId>
+ <artifactId>surefire-junit47</artifactId>
+ <version>3.0.0-M7</version>
+ </dependency>
+ </dependencies>
+ <executions>
+ <execution>
+ <id>functional-test</id>
+ <phase>integration-test</phase>
+ <goals>
+ <goal>integration-test</goal>
+ </goals>
<configuration>
- <skip>true</skip>
- </configuration>
- </plugin>
+ <!-- Turning on logs so that travis does not time out tests for not providing any output. -->
Review Comment:
Is this comment (and hack) still valid since we no longer use Travis?
##########
integration-tests-ex/cases/cluster/template.py:
##########
@@ -49,16 +49,21 @@ def generate(template_path, template):
'''
# Compute the cluster (test category) name from the template path which
- # we assume to be module/<something>/<template>/<something>.py
+ # we assume to be <module>/templates/<something>.py
template_path = Path(template_path)
- cluster = template_path.stem
+ cluster = template_path.parent.name
- # Move up to the module (that is, the cases folder) relative to the template file.
- module_dir = Path(__file__).parent.parent
+ # Move up to the module relative to the template file.
+ module_dir = template_path.parent.parent.parent
# The target location for the output file is <module>/target/cluster/<cluster>/docker-compose.yaml
target_dir = module_dir.joinpath("target")
+ os.makedirs(target_dir, exist_ok=True)
target_file = target_dir.joinpath('cluster', cluster, 'docker-compose.yaml')
+ # Uncomment the following for debugging
+ #print("template_path:", template_path)
Review Comment:
Perhaps we could use the `logging` library and add these to `DEBUG` logs rather than comment-uncomment.
##########
integration-tests-ex/docs/docker.md:
##########
@@ -211,6 +211,37 @@ when it starts. If you start, then restart the MySQL container, you *must*
remove the `db` directory before restart or MySQL will fail due to existing
files.
+### Per-test Extensions
+
+The image build includes a standard set of extensions. Contrib or custom extensions
+may wish to add additional extensions. This is most easily done not by altering the
+image, but by adding the extensions at cluster startup. If the shared directory has
+an `extensions` subdirectory, then that directory is added to the extension search
+path on container startup. To add an extension `my-extension`, your shared directory
+should look like this:
+
+```text
+shared
++- ...
++- extensions
+ +- my-extension
+ +- my-extension-<version>.jar
++- ...
+```
+
+The the `extensions` directory should be created within the per-cluster `setup.sh` script
Review Comment:
nit: "the" repeated twice
##########
docs/configuration/index.md:
##########
@@ -122,6 +122,7 @@ Many of Druid's external dependencies can be plugged in as modules. Extensions c
|`druid.extensions.useExtensionClassloaderFirst`|This is a boolean flag that determines if Druid extensions should prefer loading classes from their own jars rather than jars bundled with Druid. If false, extensions must be compatible with classes provided by any jars bundled with Druid. If true, extensions may depend on conflicting versions.|false|
|`druid.extensions.hadoopContainerDruidClasspath`|Hadoop Indexing launches hadoop jobs and this configuration provides way to explicitly set the user classpath for the hadoop job. By default this is computed automatically by druid based on the druid process classpath and set of extensions. However, sometimes you might want to be explicit to resolve dependency conflicts between druid and hadoop.|null|
|`druid.extensions.addExtensionsToHadoopContainer`|Only applicable if `druid.extensions.hadoopContainerDruidClasspath` is provided. If set to true, then extensions specified in the loadList are added to hadoop container classpath. Note that when `druid.extensions.hadoopContainerDruidClasspath` is not provided then extensions are always added to hadoop container classpath.|false|
+|`druid.extensions.path`|An optional array of paths to search for extensions. The `directory` entry acts as the first entry in the search path. Primarily for testing and Docker-based environments.|null (i.e. no additional search path)|
Review Comment:
Since this setting can hold an array of paths, should we suffix it `pathList` or `paths`?
##########
processing/src/main/java/org/apache/druid/guice/ExtensionsConfig.java:
##########
@@ -20,35 +20,63 @@
package org.apache.druid.guice;
import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.collect.Sets;
+import javax.annotation.Nullable;
import javax.validation.constraints.NotNull;
+
import java.util.LinkedHashSet;
+import java.util.List;
/**
+ * Configuration for Druid extensions.
*/
public class ExtensionsConfig
{
public static final String PROPERTY_BASE = "druid.extensions";
+ public static final String DEFAULT_EXTENSIONS_DIR = "extensions";
@JsonProperty
@NotNull
private boolean searchCurrentClassloader = true;
+ /**
+ * The location of the "primary" extensions directory. If the path is relative
+ * (as in the default), then the path is relative to the Druid directory
+ * (assuming the Druid product directory is indicated by the working directory.)
+ * <p>
+ * The value can be blank to indicate to ignore this value and only use the path.
+ * (If the value is omitted from the config file, it defaults to "extensions", so
+ * we need a blank value to say to ignore this property.)
+ */
+ @JsonProperty
+ private String directory = DEFAULT_EXTENSIONS_DIR;
+
+ /**
+ * Extensions path. Items may be relative to the Druid home directory, or absolute.
+ * Extensions are resolved in order along the path: the first match wins. If
+ * {@link #directory} is set, it becomes the first element on the full path.
+ * A typical use is to let {@code directory} point to {@code $DRUID_HOME/extensions},
+ * and use {@link path} to point to extra extensions mounted into a Docker container.
+ * Primarily for testing.
+ */
@JsonProperty
- private String directory = "extensions";
+ @Nullable
+ private List<String> path;
Review Comment:
Please see my related comment on calling this `paths` or `pathList`. It's not apparent to me without looking at its type
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] paul-rogers commented on pull request #13877: Extend the IT framework to allow tests in extensions
Posted by "paul-rogers (via GitHub)" <gi...@apache.org>.
paul-rogers commented on PR #13877:
URL: https://github.com/apache/druid/pull/13877#issuecomment-1483232879
This PR is blocked on a failure in the Kafka tests. Debugging failed to determine if a change in this PR caused the failure, or if the tests are just reliably flaky. Kafka failures seem uncorrelated to the changes made while debugging, and the tests fail so often it is hard to determine when a code path might be clean. Let's just put this on hold until the Kafka issue is resolved.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] paul-rogers commented on a diff in pull request #13877: Extend the IT framework to allow tests in extensions
Posted by "paul-rogers (via GitHub)" <gi...@apache.org>.
paul-rogers commented on code in PR #13877:
URL: https://github.com/apache/druid/pull/13877#discussion_r1128914971
##########
integration-tests-ex/cases/cluster/template.py:
##########
@@ -49,16 +49,21 @@ def generate(template_path, template):
'''
# Compute the cluster (test category) name from the template path which
- # we assume to be module/<something>/<template>/<something>.py
+ # we assume to be <module>/templates/<something>.py
template_path = Path(template_path)
- cluster = template_path.stem
+ cluster = template_path.parent.name
- # Move up to the module (that is, the cases folder) relative to the template file.
- module_dir = Path(__file__).parent.parent
+ # Move up to the module relative to the template file.
+ module_dir = template_path.parent.parent.parent
# The target location for the output file is <module>/target/cluster/<cluster>/docker-compose.yaml
target_dir = module_dir.joinpath("target")
+ os.makedirs(target_dir, exist_ok=True)
target_file = target_dir.joinpath('cluster', cluster, 'docker-compose.yaml')
+ # Uncomment the following for debugging
+ #print("template_path:", template_path)
Review Comment:
We could. But, they are only ever used when we change something. The user doesn't care: if they are wrong, things don't work, so they have to be right.
To avoid a discussion, removed the lines.
##########
docs/configuration/index.md:
##########
@@ -122,6 +122,7 @@ Many of Druid's external dependencies can be plugged in as modules. Extensions c
|`druid.extensions.useExtensionClassloaderFirst`|This is a boolean flag that determines if Druid extensions should prefer loading classes from their own jars rather than jars bundled with Druid. If false, extensions must be compatible with classes provided by any jars bundled with Druid. If true, extensions may depend on conflicting versions.|false|
|`druid.extensions.hadoopContainerDruidClasspath`|Hadoop Indexing launches hadoop jobs and this configuration provides way to explicitly set the user classpath for the hadoop job. By default this is computed automatically by druid based on the druid process classpath and set of extensions. However, sometimes you might want to be explicit to resolve dependency conflicts between druid and hadoop.|null|
|`druid.extensions.addExtensionsToHadoopContainer`|Only applicable if `druid.extensions.hadoopContainerDruidClasspath` is provided. If set to true, then extensions specified in the loadList are added to hadoop container classpath. Note that when `druid.extensions.hadoopContainerDruidClasspath` is not provided then extensions are always added to hadoop container classpath.|false|
+|`druid.extensions.path`|An optional array of paths to search for extensions. The `directory` entry acts as the first entry in the search path. Primarily for testing and Docker-based environments.|null (i.e. no additional search path)|
Review Comment:
Here I'm following the normal [Lin|U]nix pattern. In Bash, we use `PATH` for the search path. In Python, it is `PYTHONPATH`. The path, by definition, is a list.
Of course, now people say "file path" to mean a set of directories. I made that mistake here and changed it to "array of directories". But, I've not heard some one explain `PATH` as "command search paths" (with an "s").
I _could_ change it to "additionalDirectoriesArray", but I feel that is overkill.
Thoughts?
##########
processing/src/main/java/org/apache/druid/guice/ExtensionsConfig.java:
##########
@@ -20,35 +20,63 @@
package org.apache.druid.guice;
import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.collect.Sets;
+import javax.annotation.Nullable;
import javax.validation.constraints.NotNull;
+
import java.util.LinkedHashSet;
+import java.util.List;
/**
+ * Configuration for Druid extensions.
*/
public class ExtensionsConfig
{
public static final String PROPERTY_BASE = "druid.extensions";
+ public static final String DEFAULT_EXTENSIONS_DIR = "extensions";
@JsonProperty
@NotNull
private boolean searchCurrentClassloader = true;
+ /**
+ * The location of the "primary" extensions directory. If the path is relative
+ * (as in the default), then the path is relative to the Druid directory
+ * (assuming the Druid product directory is indicated by the working directory.)
+ * <p>
+ * The value can be blank to indicate to ignore this value and only use the path.
+ * (If the value is omitted from the config file, it defaults to "extensions", so
+ * we need a blank value to say to ignore this property.)
+ */
+ @JsonProperty
+ private String directory = DEFAULT_EXTENSIONS_DIR;
+
+ /**
+ * Extensions path. Items may be relative to the Druid home directory, or absolute.
+ * Extensions are resolved in order along the path: the first match wins. If
+ * {@link #directory} is set, it becomes the first element on the full path.
+ * A typical use is to let {@code directory} point to {@code $DRUID_HOME/extensions},
+ * and use {@link path} to point to extra extensions mounted into a Docker container.
+ * Primarily for testing.
+ */
@JsonProperty
- private String directory = "extensions";
+ @Nullable
+ private List<String> path;
Review Comment:
I see the point. We generally have documentation and examples since our property names don't include details. For example, `fooTimeout` often doesn't say `fooTimeoutMs`, `fooSize` doesn't say `fooSizeBytes` and `loadList` doesn't say `loadArrayOfString`.
That said, I'm open to suggestions if we feel folks can't look up the field in docs or in an example.
##########
integration-tests-ex/docs/docker.md:
##########
@@ -211,6 +211,37 @@ when it starts. If you start, then restart the MySQL container, you *must*
remove the `db` directory before restart or MySQL will fail due to existing
files.
+### Per-test Extensions
+
+The image build includes a standard set of extensions. Contrib or custom extensions
+may wish to add additional extensions. This is most easily done not by altering the
+image, but by adding the extensions at cluster startup. If the shared directory has
+an `extensions` subdirectory, then that directory is added to the extension search
+path on container startup. To add an extension `my-extension`, your shared directory
+should look like this:
+
+```text
+shared
++- ...
++- extensions
+ +- my-extension
+ +- my-extension-<version>.jar
++- ...
+```
+
+The the `extensions` directory should be created within the per-cluster `setup.sh` script
Review Comment:
Actually, it is repeated once, but appears twice. :-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] paul-rogers commented on pull request #13877: Extend the IT framework to allow tests in extensions
Posted by "paul-rogers (via GitHub)" <gi...@apache.org>.
paul-rogers commented on PR #13877:
URL: https://github.com/apache/druid/pull/13877#issuecomment-1496496100
The gRPC query PR, #14024, depends on this code. In fact, this PR was pulled out of (an early version of) the gRPC extension. Once this PR is merged, we will merge master into the rRPC PR to remove these files from the rRPC PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] github-code-scanning[bot] commented on a diff in pull request #13877: Extend the IT framework to allow tests in extensions
Posted by "github-code-scanning[bot] (via GitHub)" <gi...@apache.org>.
github-code-scanning[bot] commented on code in PR #13877:
URL: https://github.com/apache/druid/pull/13877#discussion_r1123806649
##########
processing/src/main/java/org/apache/druid/guice/ExtensionsLoader.java:
##########
@@ -109,7 +116,7 @@
}
@VisibleForTesting
- public Map<Pair<File, Boolean>, URLClassLoader> getLoadersMap()
+ public Map<String, URLClassLoader> getLoadersMap()
Review Comment:
## Exposing internal representation
getLoadersMap exposes the internal representation stored in field loaders. The value may be modified [after this call to getLoadersMap](1).
[Show more details](https://github.com/apache/druid/security/code-scanning/4368)
##########
processing/src/test/java/org/apache/druid/guice/ExtensionsLoaderTest.java:
##########
@@ -217,58 +241,121 @@
final File absolutePathExtension = temporaryFolder.newFolder();
- final ExtensionsConfig config = new ExtensionsConfig()
- {
- @Override
- public LinkedHashSet<String> getLoadList()
- {
- return Sets.newLinkedHashSet(Arrays.asList("mysql-metadata-storage", absolutePathExtension.getAbsolutePath()));
- }
-
- @Override
- public String getDirectory()
- {
- return extensionsDir.getAbsolutePath();
- }
- };
+ final ExtensionsConfig config = ExtensionsConfig.builder()
+ .directory(extensionsDir.getAbsolutePath())
+ .loadList(Arrays.asList("mysql-metadata-storage", absolutePathExtension.getAbsolutePath()))
+ .build();
final ExtensionsLoader extnLoader = new ExtensionsLoader(config);
final File mysql_metadata_storage = new File(extensionsDir, "mysql-metadata-storage");
final File random_extension = new File(extensionsDir, "random-extensions");
mysql_metadata_storage.mkdir();
random_extension.mkdir();
- final File[] expectedFileList = new File[]{mysql_metadata_storage, absolutePathExtension};
- final File[] actualFileList = extnLoader.getExtensionFilesToLoad();
- Assert.assertArrayEquals(expectedFileList, actualFileList);
+ final List<File> expectedFileList = Arrays.asList(mysql_metadata_storage, absolutePathExtension);
+ final List<File> actualFileList = extnLoader.getExtensionFilesToLoad();
+ Assert.assertEquals(expectedFileList, actualFileList);
+ }
+
+ @Test
+ public void testGetExtensionFilesToLoad_with_load_list_and_path() throws IOException
+ {
+ // The loader only looks for directories: doesn't matter if they are empty.
+ final File extensionsDir1 = temporaryFolder.newFolder();
+ String extn1 = "extension1";
+ File extn1Dir = new File(extensionsDir1, extn1);
+ extn1Dir.mkdir();
+ final File extensionsDir2 = temporaryFolder.newFolder();
+ String extn2 = "extension2";
+ File extn2Dir = new File(extensionsDir2, extn2);
+ extn2Dir.mkdir();
Review Comment:
## Ignored error status of call
Method testGetExtensionFilesToLoad_with_load_list_and_path ignores exceptional return value of File.mkdir.
[Show more details](https://github.com/apache/druid/security/code-scanning/4373)
##########
processing/src/test/java/org/apache/druid/guice/ExtensionsLoaderTest.java:
##########
@@ -217,58 +241,121 @@
final File absolutePathExtension = temporaryFolder.newFolder();
- final ExtensionsConfig config = new ExtensionsConfig()
- {
- @Override
- public LinkedHashSet<String> getLoadList()
- {
- return Sets.newLinkedHashSet(Arrays.asList("mysql-metadata-storage", absolutePathExtension.getAbsolutePath()));
- }
-
- @Override
- public String getDirectory()
- {
- return extensionsDir.getAbsolutePath();
- }
- };
+ final ExtensionsConfig config = ExtensionsConfig.builder()
+ .directory(extensionsDir.getAbsolutePath())
+ .loadList(Arrays.asList("mysql-metadata-storage", absolutePathExtension.getAbsolutePath()))
+ .build();
final ExtensionsLoader extnLoader = new ExtensionsLoader(config);
final File mysql_metadata_storage = new File(extensionsDir, "mysql-metadata-storage");
final File random_extension = new File(extensionsDir, "random-extensions");
mysql_metadata_storage.mkdir();
random_extension.mkdir();
- final File[] expectedFileList = new File[]{mysql_metadata_storage, absolutePathExtension};
- final File[] actualFileList = extnLoader.getExtensionFilesToLoad();
- Assert.assertArrayEquals(expectedFileList, actualFileList);
+ final List<File> expectedFileList = Arrays.asList(mysql_metadata_storage, absolutePathExtension);
+ final List<File> actualFileList = extnLoader.getExtensionFilesToLoad();
+ Assert.assertEquals(expectedFileList, actualFileList);
+ }
+
+ @Test
+ public void testGetExtensionFilesToLoad_with_load_list_and_path() throws IOException
+ {
+ // The loader only looks for directories: doesn't matter if they are empty.
+ final File extensionsDir1 = temporaryFolder.newFolder();
+ String extn1 = "extension1";
+ File extn1Dir = new File(extensionsDir1, extn1);
+ extn1Dir.mkdir();
Review Comment:
## Ignored error status of call
Method testGetExtensionFilesToLoad_with_load_list_and_path ignores exceptional return value of File.mkdir.
[Show more details](https://github.com/apache/druid/security/code-scanning/4372)
##########
processing/src/test/java/org/apache/druid/guice/ExtensionsLoaderTest.java:
##########
@@ -217,58 +241,121 @@
final File absolutePathExtension = temporaryFolder.newFolder();
- final ExtensionsConfig config = new ExtensionsConfig()
- {
- @Override
- public LinkedHashSet<String> getLoadList()
- {
- return Sets.newLinkedHashSet(Arrays.asList("mysql-metadata-storage", absolutePathExtension.getAbsolutePath()));
- }
-
- @Override
- public String getDirectory()
- {
- return extensionsDir.getAbsolutePath();
- }
- };
+ final ExtensionsConfig config = ExtensionsConfig.builder()
+ .directory(extensionsDir.getAbsolutePath())
+ .loadList(Arrays.asList("mysql-metadata-storage", absolutePathExtension.getAbsolutePath()))
+ .build();
final ExtensionsLoader extnLoader = new ExtensionsLoader(config);
final File mysql_metadata_storage = new File(extensionsDir, "mysql-metadata-storage");
final File random_extension = new File(extensionsDir, "random-extensions");
mysql_metadata_storage.mkdir();
random_extension.mkdir();
- final File[] expectedFileList = new File[]{mysql_metadata_storage, absolutePathExtension};
- final File[] actualFileList = extnLoader.getExtensionFilesToLoad();
- Assert.assertArrayEquals(expectedFileList, actualFileList);
+ final List<File> expectedFileList = Arrays.asList(mysql_metadata_storage, absolutePathExtension);
+ final List<File> actualFileList = extnLoader.getExtensionFilesToLoad();
+ Assert.assertEquals(expectedFileList, actualFileList);
+ }
+
+ @Test
+ public void testGetExtensionFilesToLoad_with_load_list_and_path() throws IOException
+ {
+ // The loader only looks for directories: doesn't matter if they are empty.
+ final File extensionsDir1 = temporaryFolder.newFolder();
+ String extn1 = "extension1";
+ File extn1Dir = new File(extensionsDir1, extn1);
+ extn1Dir.mkdir();
+ final File extensionsDir2 = temporaryFolder.newFolder();
+ String extn2 = "extension2";
+ File extn2Dir = new File(extensionsDir2, extn2);
+ extn2Dir.mkdir();
+
+ final ExtensionsConfig config = ExtensionsConfig.builder()
+ .directory(extensionsDir1.getAbsolutePath())
+ .path(Collections.singletonList(extensionsDir2.getAbsolutePath()))
+ .loadList(Arrays.asList(extn1, extn2))
+ .build();
+ final ExtensionsLoader extnLoader = new ExtensionsLoader(config);
+
+ final List<File> expectedFileList = Arrays.asList(extn1Dir, extn2Dir);
+ final List<File> actualFileList = extnLoader.getExtensionFilesToLoad();
+ Assert.assertEquals(expectedFileList, actualFileList);
+ }
+
+ @Test
+ public void testGetExtensionFilesToLoad_with_load_list_with_path_only() throws IOException
+ {
+ // The loader only looks for directories: doesn't matter if they are empty.
+ final File extensionsDir1 = temporaryFolder.newFolder();
+ final File extensionsDir2 = temporaryFolder.newFolder();
+
+ // For fun, mix up the order
+ String extn1 = "extension1";
+ File extn1Dir = new File(extensionsDir2, extn1);
+ extn1Dir.mkdir();
+ String extn2 = "extension2";
+ File extn2Dir = new File(extensionsDir1, extn2);
+ extn2Dir.mkdir();
Review Comment:
## Ignored error status of call
Method testGetExtensionFilesToLoad_with_load_list_with_path_only ignores exceptional return value of File.mkdir.
[Show more details](https://github.com/apache/druid/security/code-scanning/4375)
##########
processing/src/test/java/org/apache/druid/guice/ExtensionsLoaderTest.java:
##########
@@ -217,58 +241,121 @@
final File absolutePathExtension = temporaryFolder.newFolder();
- final ExtensionsConfig config = new ExtensionsConfig()
- {
- @Override
- public LinkedHashSet<String> getLoadList()
- {
- return Sets.newLinkedHashSet(Arrays.asList("mysql-metadata-storage", absolutePathExtension.getAbsolutePath()));
- }
-
- @Override
- public String getDirectory()
- {
- return extensionsDir.getAbsolutePath();
- }
- };
+ final ExtensionsConfig config = ExtensionsConfig.builder()
+ .directory(extensionsDir.getAbsolutePath())
+ .loadList(Arrays.asList("mysql-metadata-storage", absolutePathExtension.getAbsolutePath()))
+ .build();
final ExtensionsLoader extnLoader = new ExtensionsLoader(config);
final File mysql_metadata_storage = new File(extensionsDir, "mysql-metadata-storage");
final File random_extension = new File(extensionsDir, "random-extensions");
mysql_metadata_storage.mkdir();
random_extension.mkdir();
- final File[] expectedFileList = new File[]{mysql_metadata_storage, absolutePathExtension};
- final File[] actualFileList = extnLoader.getExtensionFilesToLoad();
- Assert.assertArrayEquals(expectedFileList, actualFileList);
+ final List<File> expectedFileList = Arrays.asList(mysql_metadata_storage, absolutePathExtension);
+ final List<File> actualFileList = extnLoader.getExtensionFilesToLoad();
+ Assert.assertEquals(expectedFileList, actualFileList);
+ }
+
+ @Test
+ public void testGetExtensionFilesToLoad_with_load_list_and_path() throws IOException
+ {
+ // The loader only looks for directories: doesn't matter if they are empty.
+ final File extensionsDir1 = temporaryFolder.newFolder();
+ String extn1 = "extension1";
+ File extn1Dir = new File(extensionsDir1, extn1);
+ extn1Dir.mkdir();
+ final File extensionsDir2 = temporaryFolder.newFolder();
+ String extn2 = "extension2";
+ File extn2Dir = new File(extensionsDir2, extn2);
+ extn2Dir.mkdir();
+
+ final ExtensionsConfig config = ExtensionsConfig.builder()
+ .directory(extensionsDir1.getAbsolutePath())
+ .path(Collections.singletonList(extensionsDir2.getAbsolutePath()))
+ .loadList(Arrays.asList(extn1, extn2))
+ .build();
+ final ExtensionsLoader extnLoader = new ExtensionsLoader(config);
+
+ final List<File> expectedFileList = Arrays.asList(extn1Dir, extn2Dir);
+ final List<File> actualFileList = extnLoader.getExtensionFilesToLoad();
+ Assert.assertEquals(expectedFileList, actualFileList);
+ }
+
+ @Test
+ public void testGetExtensionFilesToLoad_with_load_list_with_path_only() throws IOException
+ {
+ // The loader only looks for directories: doesn't matter if they are empty.
+ final File extensionsDir1 = temporaryFolder.newFolder();
+ final File extensionsDir2 = temporaryFolder.newFolder();
+
+ // For fun, mix up the order
+ String extn1 = "extension1";
+ File extn1Dir = new File(extensionsDir2, extn1);
+ extn1Dir.mkdir();
Review Comment:
## Ignored error status of call
Method testGetExtensionFilesToLoad_with_load_list_with_path_only ignores exceptional return value of File.mkdir.
[Show more details](https://github.com/apache/druid/security/code-scanning/4374)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] abhishekrb19 commented on a diff in pull request #13877: Extend the IT framework to allow tests in extensions
Posted by "abhishekrb19 (via GitHub)" <gi...@apache.org>.
abhishekrb19 commented on code in PR #13877:
URL: https://github.com/apache/druid/pull/13877#discussion_r1142912156
##########
docs/configuration/index.md:
##########
@@ -122,6 +122,7 @@ Many of Druid's external dependencies can be plugged in as modules. Extensions c
|`druid.extensions.useExtensionClassloaderFirst`|This is a boolean flag that determines if Druid extensions should prefer loading classes from their own jars rather than jars bundled with Druid. If false, extensions must be compatible with classes provided by any jars bundled with Druid. If true, extensions may depend on conflicting versions.|false|
|`druid.extensions.hadoopContainerDruidClasspath`|Hadoop Indexing launches hadoop jobs and this configuration provides way to explicitly set the user classpath for the hadoop job. By default this is computed automatically by druid based on the druid process classpath and set of extensions. However, sometimes you might want to be explicit to resolve dependency conflicts between druid and hadoop.|null|
|`druid.extensions.addExtensionsToHadoopContainer`|Only applicable if `druid.extensions.hadoopContainerDruidClasspath` is provided. If set to true, then extensions specified in the loadList are added to hadoop container classpath. Note that when `druid.extensions.hadoopContainerDruidClasspath` is not provided then extensions are always added to hadoop container classpath.|false|
+|`druid.extensions.path`|An optional array of paths to search for extensions. The `directory` entry acts as the first entry in the search path. Primarily for testing and Docker-based environments.|null (i.e. no additional search path)|
Review Comment:
I see, works for me. Thanks for the clarification.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org