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

support cascade execution of extraction filters in extraction dimension spec #2209

Merged
merged 1 commit into from
Jan 18, 2016

Conversation

sirpkt
Copy link
Contributor

@sirpkt sirpkt commented Jan 6, 2016

This PR fixes #1875

@sirpkt sirpkt changed the title Support chain extraction functions support cascade execution of extraction filters in extraction dimension spec Jan 6, 2016
"type" : "extraction_cascade",
"dimension" : <dimension>,
"outputName" : <output_name>,
"extractionFns" : <array of extraction_functions>
Copy link
Contributor

Choose a reason for hiding this comment

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

what about cascadeExtractionFn? How should that be used?

edit: okay I see what this is doing

Copy link
Contributor

Choose a reason for hiding this comment

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

also, can we add an example here with some sample usage?

@sirpkt
Copy link
Contributor Author

sirpkt commented Jan 8, 2016

I thought that CascadeExtractionFn is essential to support chaining in current ExtractionFn interface but it is inconvenient to use.
So, I added CascadeDimensionSpec that uses CascadeExtractionFn for only internal use and external input from users is jut array of ExtractionFn s.
However, it receives both CascadeExtractonFn(even though it is only for internal use) and array of ExtractionFn s because of serde, which makes that the patch looks unclear.

@sirpkt
Copy link
Contributor Author

sirpkt commented Jan 8, 2016

I understand what you are saying @jon-wei, @drcrallen .
So I'm gonna update the patch that CascadeExtractionFn can have array argument and handle chaining with inner functions.
Then, I can remove CasecadeExtractionDimension.


@JsonCreator
public CascadeExtractionFn(
@JsonProperty("extractionFns") ExtractionFn[] extractionFn) {
Copy link
Contributor

Choose a reason for hiding this comment

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

function parentheses/braces styling

@fjy
Copy link
Contributor

fjy commented Jan 13, 2016

@jon-wei @drcrallen more commentz?

]
}
```

Copy link
Contributor

Choose a reason for hiding this comment

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

it would be really cool if the example showed the dimension value and the final transformed value

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fjy I added an example.

@fjy
Copy link
Contributor

fjy commented Jan 15, 2016

👍 after commits squashed

@fjy
Copy link
Contributor

fjy commented Jan 15, 2016

@b-slim @jon-wei any more comments?

@jon-wei
Copy link
Contributor

jon-wei commented Jan 15, 2016

👍 after squash

@fjy
Copy link
Contributor

fjy commented Jan 15, 2016

@sirpkt will need a CLA: http://druid.io/community/cla.html

you guys may consider filling out the corporate CLA

@fjy
Copy link
Contributor

fjy commented Jan 16, 2016

@sirpkt I can merge this after commits are squashed and CLA is signed

@sirpkt sirpkt force-pushed the cascade_extraction_functions branch from cdcb7f2 to 238dd3b Compare January 18, 2016 02:11
@sirpkt
Copy link
Contributor Author

sirpkt commented Jan 18, 2016

commits are squashed.
And also I submitted CLA.

fjy added a commit that referenced this pull request Jan 18, 2016
support cascade execution of extraction filters in extraction dimension spec
@fjy fjy merged commit 592638f into apache:master Jan 18, 2016
@drcrallen drcrallen added this to the 0.9.0 milestone Feb 2, 2016
@fjy fjy mentioned this pull request Feb 5, 2016
@sirpkt sirpkt deleted the cascade_extraction_functions branch March 15, 2016 02:33
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.

Ability to chain extraction functions to expand their functionality
5 participants