Skip to content

make Coordinator IndexingService helpers pluggable#2712

Merged
pjain1 merged 1 commit intoapache:masterfrom
guobingkun:make_runnables_pluggable
Mar 28, 2016
Merged

make Coordinator IndexingService helpers pluggable#2712
pjain1 merged 1 commit intoapache:masterfrom
guobingkun:make_runnables_pluggable

Conversation

@guobingkun
Copy link
Copy Markdown
Contributor

Fixes #2682
Depends on #2724

IndexingService helpers are added according to the settings in runtime.properties.
Rather than having all the config.isXXX checks there, it makes sense to have a pluggable
approach for allowing the dynamic configuration to bring in implementations for helpers
without having to have hard-coded sets of available helpers. Plus, it will also make it possible for extensions to plug helpers in.

With druid-io/druid-api#76, we could conditionally bind a helper to Coordinator's runlist.
The condition is driven by the value set in the runtime.properties.

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.

by default what is this value?

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.

by default it is "false", it was set here

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.

i think we should maybe change this default ot true.

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.

Change of default value is outside of scope for this PR.

Comment thread pom.xml Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

there is a PR to merge druid-api 0.3.17 into druid - #2720

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.

sent #2724

@pjain1
Copy link
Copy Markdown
Member

pjain1 commented Mar 24, 2016

👍 apart from https://github.com/druid-io/druid/pull/2712/files#r57381845 and after travis

@drcrallen
Copy link
Copy Markdown
Contributor

👍 after dependency is in and travis passes.

@guobingkun guobingkun force-pushed the make_runnables_pluggable branch from 228586a to 1f1ea48 Compare March 25, 2016 16:14
Fixes apache#2682
IndexingService helpers are added according to the settings in runtime.properties.
Rather than having all the config.isXXX checks there, it makes sense to have a pluggable
approach for allowing the dynamic configuration to bring in implementations for helpers
without having to have hard-coded sets of available helpers. Plus, it will also make it possible for extensions to plug helpers in.

With druid-io/druid-api#76, we could conditionally bind a helper to Coordinator's runlist.
The condition is driven by the value set in the runtime.properties.
@guobingkun guobingkun force-pushed the make_runnables_pluggable branch from 1f1ea48 to 0872448 Compare March 25, 2016 16:49
@guobingkun
Copy link
Copy Markdown
Contributor Author

@pjain1 @drcrallen Done rebasing.

@pjain1 pjain1 merged commit 89a8277 into apache:master Mar 28, 2016
@guobingkun guobingkun deleted the make_runnables_pluggable branch March 28, 2016 17:19
guobingkun pushed a commit to guobingkun/druid that referenced this pull request Mar 30, 2016
seoeun25 pushed a commit to seoeun25/incubator-druid that referenced this pull request Jan 10, 2020
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.

4 participants