Config slurper master #191

kevinhinterlong · 2017-03-08T22:15:35Z

The config slurper allows fili to be set up in front of any instance of druid and probe druid to build metrics, dimensions, and tables.

…lurperMaster

…r style checks

…SlurperMaster

autoformatting the groovy file changed the expected string

Wiki enums are now only used in testing

…d imports stopped modifying string cases (camel/lowercase) for metrics

kevinhinterlong · 2017-03-09T13:48:27Z

fili-generic-example/src/main/java/com/yahoo/wiki/webservice/application/GenericBinder.java

+ */
+public class GenericBinder {
+    private static GenericBinder genericBinder;
+    private DruidWebService druidWebService;


This class has no longer has unused private fields

kevinhinterlong · 2017-03-09T13:51:34Z

...-example/src/main/java/com/yahoo/wiki/webservice/data/config/metric/GenericMetricLoader.java

+    public static final int DEFAULT_KILOBYTES_PER_SKETCH = 16;
+    public static final int DEFAULT_SKETCH_SIZE_IN_BYTES = DEFAULT_KILOBYTES_PER_SKETCH * BYTES_PER_KILOBYTE;
+    private static final Logger LOG = LoggerFactory.getLogger(GenericMetricLoader.class);
+    final int sketchSize;


This is copied directly from WikiMetricLoader so same issue would apply there

kevinhinterlong · 2017-03-09T13:54:09Z

...example/src/main/java/com/yahoo/wiki/webservice/data/config/dimension/GenericDimensions.java

+
+    private final SystemConfig systemConfig = SystemConfigProvider.getInstance();
+
+    private final String defaultDimensionBackendKey = systemConfig.getPackageVariableName("dimension_backend");


Also copied directly from WikiDimensions

cdeszaq · 2017-03-10T18:59:11Z

...-example/src/main/java/com/yahoo/wiki/webservice/data/config/metric/MetricNameGenerator.java

+
+    public static FiliMetricName getFiliMetricName(String name) {
+        if (defaultTimeGrain == null) {
+            throw new RuntimeException("Default time grain must be specified if no time grains are given");


This is generally an OK warning to ignore

GenericMain is not working when actually running in front of druid.

…SlurperMaster

…' into configSlurperDevelopment

michael-mclawhorn · 2017-03-14T15:29:07Z

fili-generic-example/src/main/java/com/yahoo/wiki/webservice/application/WikiMain.java

+     * @param args  command line arguments
+     * @throws Exception if the server fails to start or crashes
+     */
+    public static void main(String[] args) throws Exception {


michael-mclawhorn · 2017-03-14T15:30:41Z

fili-generic-example/src/main/java/com/yahoo/wiki/webservice/data/config/auto/ConfigLoader.java

+ * Created by kevin on 3/3/2017.
+ */
+public interface ConfigLoader {
+    public List<? extends DruidConfig> getTableNames();


Would you prefer that these get removed?

michael-mclawhorn · 2017-03-14T15:35:19Z

fili-generic-example/src/main/java/com/yahoo/wiki/webservice/data/config/auto/ConfigLoader.java

+ * Created by kevin on 3/3/2017.
+ */
+public interface ConfigLoader {
+    public List<? extends DruidConfig> getTableNames();


Ok, in general comments on classes are a good idea, and necessary on interfaces.

However, I'm not sure I understand the connection between this interface and it's name. Further, there's already a class ConfigurationLoader, which I think this interface could easily be confused with.

Also, this interface appears to be a single method function. It's possible rather than have this be an explicit interface, you could instead make anything that is currently implementing it instead to implement:

Supplier<List<? extends DruidConfig>>

https://docs.oracle.com/javase/8/docs/api/java/util/function/Supplier.html

…ong/fili into configSlurperMaster

…' into configSlurperDevelopment Merge upstream changes and fix tests

Config slurper development

kevinhinterlong · 2017-04-11T15:07:47Z

...generic-example/src/main/java/com/yahoo/wiki/webservice/data/config/auto/DruidNavigator.java

+     *
+     * @param table The TableConfig to be loaded with queries against druid.
+     */
+    private void loadTable(TableConfig table) {


These should have javadoc comments showing example json being parsed

Add expected druid responses to druid navigator

…SlurperMaster # Conflicts: # fili-core/src/main/java/com/yahoo/bard/webservice/util/IntervalUtils.java # fili-core/src/test/groovy/com/yahoo/bard/webservice/util/IntervalUtilsSpec.groovy

…SlurperMaster

kevinhinterlong · 2017-04-15T04:50:58Z

fili-generic-example/pom.xml

+        <version>0.8-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>fili-generic-example</artifactId>


Should I change the the name fili-generic-example?

kevinhinterlong · 2017-04-15T04:51:31Z

fili-generic-example/pom.xml

+                    </execution>
+                </executions>
+                <configuration>
+                    <mainClass>com.yahoo.wiki.webservice.application.GenericMain</mainClass>


Refactor .wiki

…SlurperMaster # Conflicts: # fili-core/src/main/java/com/yahoo/bard/webservice/util/IntervalUtils.java # fili-core/src/test/groovy/com/yahoo/bard/webservice/util/IntervalUtilsSpec.groovy

michael-mclawhorn · 2017-04-17T21:46:47Z

...generic-example/src/main/java/com/yahoo/wiki/webservice/data/config/auto/DruidNavigator.java

+        String url = COORDINATOR_TABLES_PATH + table.getName() + "/?full";
+        queryDruid(rootNode -> {
+            JsonNode segments = rootNode.get("segments").get(0);
+            loadMetrics(table, segments);


Make a note in class comments that the schema of the first segment is for configuration.

NPE will be thrown if the segment list is empty. Perhaps we should catch and rethrow with a better error message.

michael-mclawhorn · 2017-04-17T21:59:34Z

...-example/src/main/java/com/yahoo/wiki/webservice/data/config/metric/GenericMetricLoader.java

+     * @param metricDictionary  Metric dictionary to use for generating the metric makers.
+     */
+    protected void buildMetricMakers(MetricDictionary metricDictionary) {
+        // Create the various metric makers


Not that it's a big issue, but why is this implemented this way rather than having an initialize-once pattern?

Did you want something like this?

if (doubleSumMaker == null) { doubleSumMaker = new DoubleSumMaker(metricDictionary); }

michael-mclawhorn · 2017-04-17T22:04:31Z

...ic-example/src/main/java/com/yahoo/wiki/webservice/data/config/table/GenericTableLoader.java

+        });
+
+
+    }


extra vertical whitespace

…' into configSlurperDevelopment

…SlurperMaster

…' into configSlurperDevelopment

Config slurper development

Update for upstream changes

cdeszaq · 2017-04-20T21:00:41Z

Merged in #251 Thx for the hard work! 🎉

kevinhinterlong added 16 commits March 3, 2017 14:23

Redesign generic loader and start creating tests

51d993a

Fix unit tests for metrics/dimensions

eb50f85

Expand test, set up for loading metrics

64503c2

Add interfaces. Need to guess time grain

93af80d

Merge remote-tracking branch 'refs/remotes/yahoo/master' into configS…

daabf0e

…lurperMaster

Merge remote-tracking branch 'refs/remotes/yahoo/master'

52303e0

Only use references to interfaces

1766d91

Got most tests working. Still need to refactor and run maven tests fo…

3f2517d

…r style checks

Merge remote-tracking branch 'refs/remotes/yahoo/master'

9abe713

Merge remote-tracking branch 'refs/remotes/origin/master' into config…

83a6fb8

…SlurperMaster

Remove unnecessary code and fix tests

f030d84

autoformatting the groovy file changed the expected string

create base url for coordinator in druidNavigator

a690dc0

Rename files to Generic* instead of Wiki*

a7f3ef5

Fix url typo in DruidNavigator and refactor

2d3bb87

Wiki enums are now only used in testing

reordering

d5be970

Refactor to remove test code, Small rearrangements, remove some unuse…

bb96450

…d imports stopped modifying string cases (camel/lowercase) for metrics

kevinhinterlong commented Mar 9, 2017

View reviewed changes

cdeszaq reviewed Mar 10, 2017

View reviewed changes

kevinhinterlong added 5 commits March 11, 2017 11:09

Remove GenericBinder singleton

b4b76e4

Finish refactoring, remove unnecesary Singleton class

af23ce1

GenericMain is not working when actually running in front of druid.

Merge remote-tracking branch 'refs/remotes/yahoo/master'

f1bbbb7

Merge remote-tracking branch 'refs/remotes/origin/master' into config…

7aff9bb

…SlurperMaster

Merge remote-tracking branch 'refs/remotes/origin/configSlurperMaster…

0894bec

…' into configSlurperDevelopment

michael-mclawhorn changed the base branch from master to JekyllSync March 14, 2017 15:23

michael-mclawhorn changed the base branch from JekyllSync to master March 14, 2017 15:24

michael-mclawhorn reviewed Mar 14, 2017

View reviewed changes

kevinhinterlong and others added 3 commits April 10, 2017 20:28

Merge branch 'configSlurperMaster' of https://github.com/kevinhinterl…

bbc5720

…ong/fili into configSlurperMaster

Merge remote-tracking branch 'refs/remotes/origin/configSlurperMaster…

8886b8c

…' into configSlurperDevelopment Merge upstream changes and fix tests

Merge pull request #17 from kevinhinterlong/configSlurperDevelopment

e0480a8

Config slurper development

kevinhinterlong commented Apr 11, 2017

View reviewed changes

michael-mclawhorn assigned cdeszaq Apr 11, 2017

garyluoex self-assigned this Apr 13, 2017

kevinhinterlong and others added 5 commits April 14, 2017 22:25

Merge remote-tracking branch 'refs/remotes/yahoo/master'

ccb1b57

Add expected druid responses to druid navigator

c32785f

Merge pull request #18 from kevinhinterlong/configSlurperDevelopment

7c97a3c

Add expected druid responses to druid navigator

Merge remote-tracking branch 'refs/remotes/origin/master' into config…

b1c57cc

…SlurperMaster # Conflicts: # fili-core/src/main/java/com/yahoo/bard/webservice/util/IntervalUtils.java # fili-core/src/test/groovy/com/yahoo/bard/webservice/util/IntervalUtilsSpec.groovy

Merge remote-tracking branch 'origin/configSlurperMaster' into config…

a3cad5c

…SlurperMaster

kevinhinterlong commented Apr 15, 2017

View reviewed changes

kevinhinterlong added 2 commits April 14, 2017 23:52

Merge remote-tracking branch 'refs/remotes/origin/master' into config…

3939765

…SlurperMaster # Conflicts: # fili-core/src/main/java/com/yahoo/bard/webservice/util/IntervalUtils.java # fili-core/src/test/groovy/com/yahoo/bard/webservice/util/IntervalUtilsSpec.groovy

Fix missing import lines

2edfedd

michael-mclawhorn reviewed Apr 17, 2017

View reviewed changes

kevinhinterlong and others added 8 commits April 18, 2017 10:14

Cleanup and more comments

0ee5996

Merge remote-tracking branch 'refs/remotes/origin/configSlurperMaster…

d8b674c

…' into configSlurperDevelopment

Merge remote-tracking branch 'refs/remotes/yahoo/master'

8adb318

Merge remote-tracking branch 'refs/remotes/origin/master' into config…

ebd786a

…SlurperMaster

Merge remote-tracking branch 'refs/remotes/origin/configSlurperMaster…

f598cbc

…' into configSlurperDevelopment

Merge pull request #19 from kevinhinterlong/configSlurperDevelopment

c04817d

Config slurper development

Update for upstream changes

34c5c19

Merge pull request #20 from kevinhinterlong/configSlurperDevelopment

6739979

Update for upstream changes

cdeszaq mentioned this pull request Apr 20, 2017

Pr/191 #251

Merged

cdeszaq closed this Apr 20, 2017

kevinhinterlong deleted the configSlurperMaster branch May 26, 2017 13:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Config slurper master #191

Config slurper master #191

kevinhinterlong commented Mar 8, 2017

kevinhinterlong Mar 9, 2017

kevinhinterlong Mar 9, 2017

kevinhinterlong Mar 9, 2017

cdeszaq Mar 10, 2017

michael-mclawhorn Mar 14, 2017

michael-mclawhorn Mar 14, 2017

kevinhinterlong Mar 14, 2017

michael-mclawhorn Mar 14, 2017

kevinhinterlong Apr 11, 2017

kevinhinterlong Apr 15, 2017

kevinhinterlong Apr 15, 2017

michael-mclawhorn Apr 17, 2017

michael-mclawhorn Apr 17, 2017

kevinhinterlong Apr 18, 2017

michael-mclawhorn Apr 17, 2017

kevinhinterlong Apr 18, 2017

michael-mclawhorn Apr 17, 2017

kevinhinterlong Apr 18, 2017

cdeszaq commented Apr 20, 2017


		private final SystemConfig systemConfig = SystemConfigProvider.getInstance();

		private final String defaultDimensionBackendKey = systemConfig.getPackageVariableName("dimension_backend");

Config slurper master #191

Config slurper master #191

Conversation

kevinhinterlong commented Mar 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cdeszaq commented Apr 20, 2017