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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
