Skip to content

Move lookup docs that are in druid-proper back into lookups.md#2735

Merged
fjy merged 1 commit intoapache:masterfrom
metamx:fixlookupDocs
Mar 26, 2016
Merged

Move lookup docs that are in druid-proper back into lookups.md#2735
fjy merged 1 commit intoapache:masterfrom
metamx:fixlookupDocs

Conversation

@drcrallen
Copy link
Copy Markdown
Contributor

No description provided.

@fjy
Copy link
Copy Markdown
Contributor

fjy commented Mar 25, 2016

@drcrallen Is dynamic configuration not a part of any extension?

Comment thread docs/content/querying/lookups.md Outdated
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.

can we use the note-caution div to indicate experimental?

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.

I can, but don't know the syntax you're looking for, is there another doc that uses that indication?

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.

ah, found it

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.

fixed

@fjy
Copy link
Copy Markdown
Contributor

fjy commented Mar 25, 2016

@drcrallen this will have to be backported

@drcrallen
Copy link
Copy Markdown
Contributor Author

Dynamic config is not an extension

@drcrallen
Copy link
Copy Markdown
Contributor Author

@fjy well, this feature is in 0.9.1 not 0.9.0

@fjy
Copy link
Copy Markdown
Contributor

fjy commented Mar 25, 2016

@drcrallen okay

can we remove it from the 0.9.0 docs then?

@drcrallen
Copy link
Copy Markdown
Contributor Author

@gianm
Copy link
Copy Markdown
Contributor

gianm commented Mar 25, 2016

@drcrallen is the namespaced lookup extension no longer suggested for use in 0.9.1, in favor of the dynamic config stuff?

@drcrallen
Copy link
Copy Markdown
Contributor Author

@gianm that is still being developed. @sirpkt beat me to the punch on this part: #2716

Once #2716 is fleshed out a little more we'll know for sure. But right now there is not enough change in master to make the change in recommendation.

@gianm
Copy link
Copy Markdown
Contributor

gianm commented Mar 25, 2016

ok, I'm asking because this PR removes references to the namespaced-lookups extension from lookups.md. I feel like we should have some stuff on both lookups.md and the namespaced lookup extension docs that clarify the situation for readers.

@gianm
Copy link
Copy Markdown
Contributor

gianm commented Mar 25, 2016

@drcrallen In other words: I am happy with this PR to the degree that you are happy with how lookups.md and namespaced-lookup.md make it clear to readers what the options are for cluster-wide server-side query time lookups, and what the most recommended option is.

@drcrallen
Copy link
Copy Markdown
Contributor Author

@gianm ah good point, I read

For lookups defined cluster-wide rather than embedded in the query, please look at the namespaced lookup extension.

and thought of the cluster wide config. I didn't think of the static config in the extension. I'll add that line back in.

@drcrallen
Copy link
Copy Markdown
Contributor Author

@gianm changed to

For static lookups defined in runtime.properties rather than embedded in the query, please look at the experimental namespaced lookup extension.

@gianm
Copy link
Copy Markdown
Contributor

gianm commented Mar 25, 2016

👍 looks good to me, thanks for catching this.

@gianm gianm closed this Mar 25, 2016
@gianm gianm reopened this Mar 25, 2016
@fjy fjy closed this Mar 26, 2016
@fjy fjy reopened this Mar 26, 2016
@fjy fjy merged commit a0216dc into apache:master Mar 26, 2016
@xvrl xvrl deleted the fixlookupDocs branch April 12, 2016 16:01
@xvrl xvrl added this to the 0.9.1 milestone Apr 12, 2016
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