Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use all available segment metadata for fili-generic-example #445

Merged
merged 2 commits into from
Aug 14, 2017

Conversation

kevinhinterlong
Copy link
Member

@kevinhinterlong kevinhinterlong commented Jul 25, 2017

  • The fili-generic-example was previously only using the first segment in the response from Druid but it may as well use all of them
  • This also provides these segments to the MetadataService

Addresses #428

Copy link
Contributor

@michael-mclawhorn michael-mclawhorn left a comment

Choose a reason for hiding this comment

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

This looks good. Have you tested in locally?

@kevinhinterlong
Copy link
Member Author

@michael-mclawhorn yes

Copy link
Collaborator

@garyluoex garyluoex left a comment

Choose a reason for hiding this comment

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

Minor

@@ -34,14 +37,14 @@
*
* @return a list of names of metrics for the current datasource.
Copy link
Collaborator

Choose a reason for hiding this comment

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

comment out of date.

@@ -34,14 +37,14 @@
*
* @return a list of names of metrics for the current datasource.
*/
List<String> getMetrics();
Set<String> getMetrics();

/**
* Gets the names of all the dimensions for the current datasource.
*
* @return a list of names of dimensions for the current datasource.
Copy link
Collaborator

Choose a reason for hiding this comment

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

same

- The fili-generic-example was previously only using the first segment but it may
as well use all
- This also provides these segments to the MetadataService
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants