Skip to content

Fix router documentation for druid.router.sql.enable#11716

Merged
abhishekagarwal87 merged 3 commits intoapache:masterfrom
kfaraz:fix_router_doc
Sep 28, 2021
Merged

Fix router documentation for druid.router.sql.enable#11716
abhishekagarwal87 merged 3 commits intoapache:masterfrom
kfaraz:fix_router_doc

Conversation

@kfaraz
Copy link
Contributor

@kfaraz kfaraz commented Sep 16, 2021

Changes

  • Rename field AsyncQueryForwardingServlet.routeSqlQueries to routeSqlByStrategy
  • Fix router documentation to remove ambiguity

This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added or updated version, license, or notice information in licenses.yaml
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • added integration tests.
  • been tested in a test Druid cluster.

|`druid.router.defaultRule`|The default rule for all datasources.|"_default"|
|`druid.router.pollPeriod`|How often to poll for new rules.|PT1M|
|`druid.router.sql.enable`|Enable routing of SQL queries. Possible values are `true` and `false`. When set to `true`, the Router uses the provided strategies to determine the broker service for a given SQL query.|`false`|
|`druid.router.sql.enable`|Enable routing of SQL queries using strategies. Possible values are `true` and `false`. When set to `true`, the Router uses the provided strategies to determine the broker service for a given SQL query. When set to `false`, the Router uses the `defaultBrokerServiceName`.|`false`|
Copy link
Contributor

Choose a reason for hiding this comment

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

can you also add what strategies are supported?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The details of strategies are present in docs/design/router.md. I figured we only need a summary here.

Copy link
Contributor

@techdocsmith techdocsmith left a comment

Choose a reason for hiding this comment

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

Request some style changes, minor clarifications.

Co-authored-by: Charles Smith <techdocsmith@gmail.com>
@kfaraz
Copy link
Contributor Author

kfaraz commented Sep 24, 2021

Thanks a lot for the review, @techdocsmith ! I have committed your suggestions.

Copy link
Contributor

@techdocsmith techdocsmith left a comment

Choose a reason for hiding this comment

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

LGTM

@abhishekagarwal87 abhishekagarwal87 merged commit c641657 into apache:master Sep 28, 2021
@abhishekagarwal87
Copy link
Contributor

Thank you @kfaraz. I have merged the PR.

@abhishekagarwal87 abhishekagarwal87 added this to the 0.23.0 milestone May 11, 2022
@kfaraz kfaraz deleted the fix_router_doc branch August 1, 2023 04:53
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