You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2020/02/17 16:12:37 UTC
[GitHub] [druid] bjozet opened a new pull request #9372: Add config option
for namespacePrefix
bjozet opened a new pull request #9372: Add config option for namespacePrefix
URL: https://github.com/apache/druid/pull/9372
opentsdb emitter sends metric names to opentsdb verbatim as what druid
names them, for example "query.count", this doesn't fit well with a
central opentsdb server which might have namespaced metrics, for example
"druid.query.count". This adds support for adding an optional prefix.
The prefix also gets a trailing dot (.), after it, so the metric name
becomes <namespacePrefix>.<metricname>
configureable as "druid.emitter.opentsdb.namespacePrefix", as
documented.
Co-authored-by: Martin Gerholm <ma...@deltaprojects.com>
Signed-off-by: Martin Gerholm <ma...@deltaprojects.com>
Signed-off-by: Björn Zettergren <bj...@deltaprojects.com>
<hr>
This PR has:
- [x] been self-reviewed.
- [x] added documentation for new or modified features or behaviors.
- [x] added unit tests or modified existing tests to cover new code paths.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] clintropolis commented on a change in pull request #9372:
Add config option for namespacePrefix
Posted by GitBox <gi...@apache.org>.
clintropolis commented on a change in pull request #9372: Add config option for namespacePrefix
URL: https://github.com/apache/druid/pull/9372#discussion_r381075918
##########
File path: extensions-contrib/opentsdb-emitter/src/main/java/org/apache/druid/emitter/opentsdb/EventConverter.java
##########
@@ -41,17 +41,24 @@
private static final Pattern WHITESPACE = Pattern.compile("[\\s]+");
private final Map<String, Set<String>> metricMap;
+ private final String namespacePrefix;
- public EventConverter(ObjectMapper mapper, String metricMapPath)
+ public EventConverter(ObjectMapper mapper, String metricMapPath, String namespacePrefix)
{
metricMap = readMap(mapper, metricMapPath);
+ this.namespacePrefix = namespacePrefix;
}
protected String sanitize(String metric)
{
return WHITESPACE.matcher(metric.trim()).replaceAll("_").replace('/', '.');
}
+ private String buildNamespace()
+ {
+ return (namespacePrefix == null || "".equals(namespacePrefix)) ? "" : sanitize(namespacePrefix) + ".";
Review comment:
can `namespacePrefix` actually be empty here (other than the unit tests creating an `EventConverter` directly) since `OpentsdbEmitterConfig` looks like it will coerce the value to `null` in its constructor?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] bjozet commented on issue #9372: Add config option for
namespacePrefix
Posted by GitBox <gi...@apache.org>.
bjozet commented on issue #9372: Add config option for namespacePrefix
URL: https://github.com/apache/druid/pull/9372#issuecomment-587158123
One travis check that fails is `mdspell` saying that: "namespacePrefix" is misspelled. It's the variable/configitems name, how would i circumvent this?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] bjozet commented on a change in pull request #9372: Add
config option for namespacePrefix
Posted by GitBox <gi...@apache.org>.
bjozet commented on a change in pull request #9372: Add config option for namespacePrefix
URL: https://github.com/apache/druid/pull/9372#discussion_r381208619
##########
File path: extensions-contrib/opentsdb-emitter/src/main/java/org/apache/druid/emitter/opentsdb/EventConverter.java
##########
@@ -41,17 +41,24 @@
private static final Pattern WHITESPACE = Pattern.compile("[\\s]+");
private final Map<String, Set<String>> metricMap;
+ private final String namespacePrefix;
- public EventConverter(ObjectMapper mapper, String metricMapPath)
+ public EventConverter(ObjectMapper mapper, String metricMapPath, String namespacePrefix)
{
metricMap = readMap(mapper, metricMapPath);
+ this.namespacePrefix = namespacePrefix;
}
protected String sanitize(String metric)
{
return WHITESPACE.matcher(metric.trim()).replaceAll("_").replace('/', '.');
}
+ private String buildNamespace()
Review comment:
Makes perfect sense, i've added `buildMetric` more or like what you suggested.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] QiuMM commented on a change in pull request #9372: Add
config option for namespacePrefix
Posted by GitBox <gi...@apache.org>.
QiuMM commented on a change in pull request #9372: Add config option for namespacePrefix
URL: https://github.com/apache/druid/pull/9372#discussion_r381146557
##########
File path: extensions-contrib/opentsdb-emitter/src/test/java/org/apache/druid/emitter/opentsdb/EventConverterTest.java
##########
@@ -32,23 +32,29 @@
public class EventConverterTest
{
- private EventConverter converter;
+ private EventConverter converterWithPrefix;
+ private EventConverter converterWithSpacePrefix;
+ private EventConverter converterWithoutPrefix;
@Before
public void setUp()
{
- converter = new EventConverter(new ObjectMapper(), null);
+ converterWithPrefix = new EventConverter(new ObjectMapper(), null, "druid");
+ converterWithSpacePrefix = new EventConverter(new ObjectMapper(), null, "legendary druid");
Review comment:
Sounds good.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] bjozet commented on a change in pull request #9372: Add
config option for namespacePrefix
Posted by GitBox <gi...@apache.org>.
bjozet commented on a change in pull request #9372: Add config option for namespacePrefix
URL: https://github.com/apache/druid/pull/9372#discussion_r381958484
##########
File path: extensions-contrib/opentsdb-emitter/src/test/java/org/apache/druid/emitter/opentsdb/OpentsdbEmitterConfigTest.java
##########
@@ -39,10 +39,41 @@ public void setUp()
@Test
public void testSerDeserOpentsdbEmitterConfig() throws Exception
{
- OpentsdbEmitterConfig opentsdbEmitterConfig = new OpentsdbEmitterConfig("localhost", 9999, 2000, 2000, 200, 2000, 10000L, null);
+ OpentsdbEmitterConfig opentsdbEmitterConfig = new OpentsdbEmitterConfig("localhost", 9999, 2000, 2000, 200, 2000, 10000L, null, "druid");
String opentsdbEmitterConfigString = mapper.writeValueAsString(opentsdbEmitterConfig);
OpentsdbEmitterConfig expectedOpentsdbEmitterConfig = mapper.readerFor(OpentsdbEmitterConfig.class)
.readValue(opentsdbEmitterConfigString);
Assert.assertEquals(expectedOpentsdbEmitterConfig, opentsdbEmitterConfig);
}
+
+ @Test
+ public void testSerDeserOpentsdbEmitterConfigWithSpaceNamespace() throws Exception
Review comment:
Well spotted, didn't think much of test names when refactoring. And actually, I should bring all test-names in line with same naming convention, for clarity. In this file the tests were named like `<...>Namespace<...>`, and in `EventConverterTest.java` they're called `<...>Prefix<...>`, they should all be `<...>NamespacePrefix<...>`. I'll update them all. :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] QiuMM commented on a change in pull request #9372: Add
config option for namespacePrefix
Posted by GitBox <gi...@apache.org>.
QiuMM commented on a change in pull request #9372: Add config option for namespacePrefix
URL: https://github.com/apache/druid/pull/9372#discussion_r381826855
##########
File path: extensions-contrib/opentsdb-emitter/src/test/java/org/apache/druid/emitter/opentsdb/OpentsdbEmitterConfigTest.java
##########
@@ -39,10 +39,41 @@ public void setUp()
@Test
public void testSerDeserOpentsdbEmitterConfig() throws Exception
{
- OpentsdbEmitterConfig opentsdbEmitterConfig = new OpentsdbEmitterConfig("localhost", 9999, 2000, 2000, 200, 2000, 10000L, null);
+ OpentsdbEmitterConfig opentsdbEmitterConfig = new OpentsdbEmitterConfig("localhost", 9999, 2000, 2000, 200, 2000, 10000L, null, "druid");
String opentsdbEmitterConfigString = mapper.writeValueAsString(opentsdbEmitterConfig);
OpentsdbEmitterConfig expectedOpentsdbEmitterConfig = mapper.readerFor(OpentsdbEmitterConfig.class)
.readValue(opentsdbEmitterConfigString);
Assert.assertEquals(expectedOpentsdbEmitterConfig, opentsdbEmitterConfig);
}
+
+ @Test
+ public void testSerDeserOpentsdbEmitterConfigWithSpaceNamespace() throws Exception
Review comment:
nit: rename `testSerDeserOpentsdbEmitterConfigWithSpaceNamespace` to `testSerDeserOpentsdbEmitterConfigWithNamespaceContainingSpace`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] clintropolis commented on a change in pull request #9372:
Add config option for namespacePrefix
Posted by GitBox <gi...@apache.org>.
clintropolis commented on a change in pull request #9372: Add config option for namespacePrefix
URL: https://github.com/apache/druid/pull/9372#discussion_r381652310
##########
File path: extensions-contrib/opentsdb-emitter/src/main/java/org/apache/druid/emitter/opentsdb/EventConverter.java
##########
@@ -54,9 +55,14 @@ protected String sanitize(String metric)
return WHITESPACE.matcher(metric.trim()).replaceAll("_").replace('/', '.');
}
- private String buildNamespace()
+ private String buildMetric(String metric)
{
- return (namespacePrefix == null || "".equals(namespacePrefix)) ? "" : sanitize(namespacePrefix) + ".";
+ final String sanitized = sanitize(metric);
+ if (Strings.isNullOrEmpty(namespacePrefix)) {
Review comment:
I think this can just check `namespacePrefix == null` now
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] clintropolis commented on issue #9372: Add config option
for namespacePrefix
Posted by GitBox <gi...@apache.org>.
clintropolis commented on issue #9372: Add config option for namespacePrefix
URL: https://github.com/apache/druid/pull/9372#issuecomment-588565851
>Have addressed the issues raised, recieved two "lgtm", should I squash into a single commit? :)
no need to squash, that will happen when we merge the 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] bjozet commented on a change in pull request #9372: Add
config option for namespacePrefix
Posted by GitBox <gi...@apache.org>.
bjozet commented on a change in pull request #9372: Add config option for namespacePrefix
URL: https://github.com/apache/druid/pull/9372#discussion_r381986263
##########
File path: extensions-contrib/opentsdb-emitter/src/main/java/org/apache/druid/emitter/opentsdb/EventConverter.java
##########
@@ -54,9 +55,14 @@ protected String sanitize(String metric)
return WHITESPACE.matcher(metric.trim()).replaceAll("_").replace('/', '.');
}
- private String buildNamespace()
+ private String buildMetric(String metric)
{
- return (namespacePrefix == null || "".equals(namespacePrefix)) ? "" : sanitize(namespacePrefix) + ".";
+ final String sanitized = sanitize(metric);
+ if (Strings.isNullOrEmpty(namespacePrefix)) {
Review comment:
Fixed in latest commit
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] bjozet commented on a change in pull request #9372: Add
config option for namespacePrefix
Posted by GitBox <gi...@apache.org>.
bjozet commented on a change in pull request #9372: Add config option for namespacePrefix
URL: https://github.com/apache/druid/pull/9372#discussion_r381209070
##########
File path: extensions-contrib/opentsdb-emitter/src/main/java/org/apache/druid/emitter/opentsdb/EventConverter.java
##########
@@ -41,17 +41,24 @@
private static final Pattern WHITESPACE = Pattern.compile("[\\s]+");
private final Map<String, Set<String>> metricMap;
+ private final String namespacePrefix;
- public EventConverter(ObjectMapper mapper, String metricMapPath)
+ public EventConverter(ObjectMapper mapper, String metricMapPath, String namespacePrefix)
{
metricMap = readMap(mapper, metricMapPath);
+ this.namespacePrefix = namespacePrefix;
}
protected String sanitize(String metric)
{
return WHITESPACE.matcher(metric.trim()).replaceAll("_").replace('/', '.');
}
+ private String buildNamespace()
+ {
+ return (namespacePrefix == null || "".equals(namespacePrefix)) ? "" : sanitize(namespacePrefix) + ".";
Review comment:
You're right, i've removed the tests for it, since it's the only place where it can happen. And i guess no-one is gonna call `EventConverter` outside of the extension.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] QiuMM commented on a change in pull request #9372: Add
config option for namespacePrefix
Posted by GitBox <gi...@apache.org>.
QiuMM commented on a change in pull request #9372: Add config option for namespacePrefix
URL: https://github.com/apache/druid/pull/9372#discussion_r380587513
##########
File path: extensions-contrib/opentsdb-emitter/src/test/java/org/apache/druid/emitter/opentsdb/EventConverterTest.java
##########
@@ -32,23 +32,29 @@
public class EventConverterTest
{
- private EventConverter converter;
+ private EventConverter converterWithPrefix;
+ private EventConverter converterWithSpacePrefix;
+ private EventConverter converterWithoutPrefix;
@Before
public void setUp()
{
- converter = new EventConverter(new ObjectMapper(), null);
+ converterWithPrefix = new EventConverter(new ObjectMapper(), null, "druid");
+ converterWithSpacePrefix = new EventConverter(new ObjectMapper(), null, "legendary druid");
Review comment:
Space prefix ? Maybe you should change `"legendary druid"` to `""`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] bjozet commented on a change in pull request #9372: Add
config option for namespacePrefix
Posted by GitBox <gi...@apache.org>.
bjozet commented on a change in pull request #9372: Add config option for namespacePrefix
URL: https://github.com/apache/druid/pull/9372#discussion_r381986340
##########
File path: extensions-contrib/opentsdb-emitter/src/test/java/org/apache/druid/emitter/opentsdb/OpentsdbEmitterConfigTest.java
##########
@@ -39,10 +39,41 @@ public void setUp()
@Test
public void testSerDeserOpentsdbEmitterConfig() throws Exception
{
- OpentsdbEmitterConfig opentsdbEmitterConfig = new OpentsdbEmitterConfig("localhost", 9999, 2000, 2000, 200, 2000, 10000L, null);
+ OpentsdbEmitterConfig opentsdbEmitterConfig = new OpentsdbEmitterConfig("localhost", 9999, 2000, 2000, 200, 2000, 10000L, null, "druid");
String opentsdbEmitterConfigString = mapper.writeValueAsString(opentsdbEmitterConfig);
OpentsdbEmitterConfig expectedOpentsdbEmitterConfig = mapper.readerFor(OpentsdbEmitterConfig.class)
.readValue(opentsdbEmitterConfigString);
Assert.assertEquals(expectedOpentsdbEmitterConfig, opentsdbEmitterConfig);
}
+
+ @Test
+ public void testSerDeserOpentsdbEmitterConfigWithSpaceNamespace() throws Exception
Review comment:
committed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] bjozet removed a comment on issue #9372: Add config option
for namespacePrefix
Posted by GitBox <gi...@apache.org>.
bjozet removed a comment on issue #9372: Add config option for namespacePrefix
URL: https://github.com/apache/druid/pull/9372#issuecomment-587158123
One travis check that fails is `mdspell` saying that: "namespacePrefix" is misspelled. It's the variable/configitems name, how would i circumvent this?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] QiuMM commented on a change in pull request #9372: Add
config option for namespacePrefix
Posted by GitBox <gi...@apache.org>.
QiuMM commented on a change in pull request #9372: Add config option for namespacePrefix
URL: https://github.com/apache/druid/pull/9372#discussion_r381826855
##########
File path: extensions-contrib/opentsdb-emitter/src/test/java/org/apache/druid/emitter/opentsdb/OpentsdbEmitterConfigTest.java
##########
@@ -39,10 +39,41 @@ public void setUp()
@Test
public void testSerDeserOpentsdbEmitterConfig() throws Exception
{
- OpentsdbEmitterConfig opentsdbEmitterConfig = new OpentsdbEmitterConfig("localhost", 9999, 2000, 2000, 200, 2000, 10000L, null);
+ OpentsdbEmitterConfig opentsdbEmitterConfig = new OpentsdbEmitterConfig("localhost", 9999, 2000, 2000, 200, 2000, 10000L, null, "druid");
String opentsdbEmitterConfigString = mapper.writeValueAsString(opentsdbEmitterConfig);
OpentsdbEmitterConfig expectedOpentsdbEmitterConfig = mapper.readerFor(OpentsdbEmitterConfig.class)
.readValue(opentsdbEmitterConfigString);
Assert.assertEquals(expectedOpentsdbEmitterConfig, opentsdbEmitterConfig);
}
+
+ @Test
+ public void testSerDeserOpentsdbEmitterConfigWithSpaceNamespace() throws Exception
Review comment:
nit: I think rename `testSerDeserOpentsdbEmitterConfigWithSpaceNamespace` to `testSerDeserOpentsdbEmitterConfigWithNamespaceContainingSpace` would be better.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] clintropolis commented on a change in pull request #9372:
Add config option for namespacePrefix
Posted by GitBox <gi...@apache.org>.
clintropolis commented on a change in pull request #9372: Add config option for namespacePrefix
URL: https://github.com/apache/druid/pull/9372#discussion_r381074927
##########
File path: extensions-contrib/opentsdb-emitter/src/main/java/org/apache/druid/emitter/opentsdb/EventConverter.java
##########
@@ -41,17 +41,24 @@
private static final Pattern WHITESPACE = Pattern.compile("[\\s]+");
private final Map<String, Set<String>> metricMap;
+ private final String namespacePrefix;
- public EventConverter(ObjectMapper mapper, String metricMapPath)
+ public EventConverter(ObjectMapper mapper, String metricMapPath, String namespacePrefix)
{
metricMap = readMap(mapper, metricMapPath);
+ this.namespacePrefix = namespacePrefix;
}
protected String sanitize(String metric)
{
return WHITESPACE.matcher(metric.trim()).replaceAll("_").replace('/', '.');
}
+ private String buildNamespace()
Review comment:
nit: I think this might be a fair bit cleaner to rework this method to take the metric name and do the call to `sanitize` in here, maybe something like:
```
private String buildMetric(String metric)
{
final String sanitized = sanitize(metric);
if (Strings.isNullOrEmpty(namespacePrefix)) {
return sanitized;
} else {
return StringUtils.format("%s.%s", namespacePrefix, sanitized);
}
}
```
then creating the `OpentsdbEvent` can just be
```
...
return new OpentsdbEvent(buildMetric(metric), timestamp, value, tags);
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] bjozet commented on a change in pull request #9372: Add
config option for namespacePrefix
Posted by GitBox <gi...@apache.org>.
bjozet commented on a change in pull request #9372: Add config option for namespacePrefix
URL: https://github.com/apache/druid/pull/9372#discussion_r380634727
##########
File path: extensions-contrib/opentsdb-emitter/src/test/java/org/apache/druid/emitter/opentsdb/EventConverterTest.java
##########
@@ -32,23 +32,29 @@
public class EventConverterTest
{
- private EventConverter converter;
+ private EventConverter converterWithPrefix;
+ private EventConverter converterWithSpacePrefix;
+ private EventConverter converterWithoutPrefix;
@Before
public void setUp()
{
- converter = new EventConverter(new ObjectMapper(), null);
+ converterWithPrefix = new EventConverter(new ObjectMapper(), null, "druid");
+ converterWithSpacePrefix = new EventConverter(new ObjectMapper(), null, "legendary druid");
Review comment:
You're right, i'll rename the variable to `converterWithPrefixContainingSpace`, and add another test for `converterWithPrefixContainingEmptyString` ("").
How does that sound?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] clintropolis merged pull request #9372: Add config option
for namespacePrefix
Posted by GitBox <gi...@apache.org>.
clintropolis merged pull request #9372: Add config option for namespacePrefix
URL: https://github.com/apache/druid/pull/9372
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] QiuMM commented on a change in pull request #9372: Add
config option for namespacePrefix
Posted by GitBox <gi...@apache.org>.
QiuMM commented on a change in pull request #9372: Add config option for namespacePrefix
URL: https://github.com/apache/druid/pull/9372#discussion_r380587513
##########
File path: extensions-contrib/opentsdb-emitter/src/test/java/org/apache/druid/emitter/opentsdb/EventConverterTest.java
##########
@@ -32,23 +32,29 @@
public class EventConverterTest
{
- private EventConverter converter;
+ private EventConverter converterWithPrefix;
+ private EventConverter converterWithSpacePrefix;
+ private EventConverter converterWithoutPrefix;
@Before
public void setUp()
{
- converter = new EventConverter(new ObjectMapper(), null);
+ converterWithPrefix = new EventConverter(new ObjectMapper(), null, "druid");
+ converterWithSpacePrefix = new EventConverter(new ObjectMapper(), null, "legendary druid");
Review comment:
Space prefix ? Maybe you should change `"legendary druid"` to `""` or rename the variable.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] bjozet commented on issue #9372: Add config option for
namespacePrefix
Posted by GitBox <gi...@apache.org>.
bjozet commented on issue #9372: Add config option for namespacePrefix
URL: https://github.com/apache/druid/pull/9372#issuecomment-588304692
Have addressed the issues raised, recieved two "lgtm", should I squash into a single commit? :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org