Skip to content

Support use of DimensionSchema class in DimensionsSpec#2607

Merged
himanshug merged 1 commit intoapache:masterfrom
jon-wei:dim_schema
Mar 22, 2016
Merged

Support use of DimensionSchema class in DimensionsSpec#2607
himanshug merged 1 commit intoapache:masterfrom
jon-wei:dim_schema

Conversation

@jon-wei
Copy link
Copy Markdown
Contributor

@jon-wei jon-wei commented Mar 8, 2016

Updates druid to support the change made in this druid-api PR: https://github.com/druid-io/druid-api/pull/75/files

Needs druid-api version update after that PR is merged

The DimensionsSpec object now represents dimensions with a DimensionSchema class, not as String names, to support additional dimension types.

@jon-wei
Copy link
Copy Markdown
Contributor Author

jon-wei commented Mar 8, 2016

I have a much larger PR that I am cleaning up right now that adds Long/Float support along with a new generic dimension handling interface.

I'm opening this PR now to cut down the size of that later patch, for easier review.

@jon-wei jon-wei force-pushed the dim_schema branch 2 times, most recently from 48ebf4d to 63077f0 Compare March 9, 2016 21:48
@jon-wei jon-wei added this to the 0.9.1 milestone Mar 9, 2016
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: can "spatial" be made a static final variable in DimensionSchema ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@himanshug updated this and the other patch with static finals

@gianm
Copy link
Copy Markdown
Contributor

gianm commented Mar 15, 2016

@jon-wei could you take a look at the compilation failures?

@gianm
Copy link
Copy Markdown
Contributor

gianm commented Mar 15, 2016

@jon-wei or are these expected due to not having the new druid-api yet?

@gianm
Copy link
Copy Markdown
Contributor

gianm commented Mar 15, 2016

nevermind, they're definitely expected.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it is gonna be hella confusing to understand what is a dim or metric here, can we distinguish them some other way?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@fjy Changed this to an Object -> ValueType map with comments explaining the use cases

@fjy
Copy link
Copy Markdown
Contributor

fjy commented Mar 16, 2016

@jon-wei mostly looks good, only 1 comment

can we also update druid-api version in this PR?

@jon-wei jon-wei force-pushed the dim_schema branch 2 times, most recently from 3962d6b to e07f8e4 Compare March 16, 2016 21:58
@jon-wei
Copy link
Copy Markdown
Contributor Author

jon-wei commented Mar 16, 2016

@fjy updated druid-api to 3.17 in the pom

@fjy fjy closed this Mar 21, 2016
@fjy fjy reopened this Mar 21, 2016
@fjy
Copy link
Copy Markdown
Contributor

fjy commented Mar 21, 2016

👍

@jon-wei jon-wei closed this Mar 21, 2016
@jon-wei jon-wei reopened this Mar 21, 2016
@fjy
Copy link
Copy Markdown
Contributor

fjy commented Mar 21, 2016

@jon-wei this is sitll failing UT

@jon-wei
Copy link
Copy Markdown
Contributor Author

jon-wei commented Mar 21, 2016

@fjy rebased, should pass travis now

himanshug added a commit that referenced this pull request Mar 22, 2016
Support use of DimensionSchema class in DimensionsSpec
@himanshug himanshug merged commit 00d7021 into apache:master Mar 22, 2016
@jon-wei jon-wei deleted the dim_schema branch October 6, 2017 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants