Skip to content

Remove stale 'namespace' config for JDBC lookups from doc#10886

Merged
jihoonson merged 5 commits intoapache:masterfrom
jihoonson:jdbc-lookup-namespace
Mar 5, 2021
Merged

Remove stale 'namespace' config for JDBC lookups from doc#10886
jihoonson merged 5 commits intoapache:masterfrom
jihoonson:jdbc-lookup-namespace

Conversation

@jihoonson
Copy link
Copy Markdown
Contributor

Description

The namespace config was removed in #2926. Also fixed a wrong instruction for the JDBC connector location.


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.

},

// JDBC stuff
{
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.

From looking at Travis failure, It looks like this requires a snapshot update. jest --updateSnapshot

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.

Thanks for the review. I reverted this change in favor of #10888.

Copy link
Copy Markdown
Contributor

@capistrant capistrant left a comment

Choose a reason for hiding this comment

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

LGTM outside of the spellchecker flag by CI

> For MySQL, you can get it from https://dev.mysql.com/downloads/connector/j/.
> Copy or symlink the downloaded file to `extensions/druid-lookups-cached-global` under the distribution root directory.
> The connector JAR should locate in the classpath of Druid's main class loader.
> An easy way to add it in the classpath is copying or symlinking the downloaded file to `lib/` under the distribution root directory.
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.

looks like "symlinking" is called out by spell checker. I think it would be fine to add it to the dictionary in the website module. Might not be in a common dictionary, but from a computing standpoint it is a valid word IMO.

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 agree it's valid, but deleted it by addressing @techdocsmith's comment below.

@jihoonson jihoonson changed the title Remove stale 'namespace' config for JDBC lookups from doc and web-console Remove stale 'namespace' config for JDBC lookups from doc Feb 18, 2021
> For Postgres, the connector JAR is already included.
> For MySQL, you can get it from https://dev.mysql.com/downloads/connector/j/.
> Copy or symlink the downloaded file to `extensions/druid-lookups-cached-global` under the distribution root directory.
> The connector JAR should locate in the classpath of Druid's main class loader.
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.

Suggested change
> The connector JAR should locate in the classpath of Druid's main class loader.
> The connector JAR should reside in the classpath of Druid's main class loader.

reside or "be located?"

> For MySQL, you can get it from https://dev.mysql.com/downloads/connector/j/.
> Copy or symlink the downloaded file to `extensions/druid-lookups-cached-global` under the distribution root directory.
> The connector JAR should locate in the classpath of Druid's main class loader.
> An easy way to add it in the classpath is copying or symlinking the downloaded file to `lib/` under the distribution root directory.
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.

Suggested change
> An easy way to add it in the classpath is copying or symlinking the downloaded file to `lib/` under the distribution root directory.
> To add the connector JAR to the classpath, you can copy the downloaded file to `lib/` under the distribution root directory. Alternatively, create a symbolic link to the connector in the `lib` directory.

@jihoonson
Copy link
Copy Markdown
Contributor Author

@techdocsmith thanks, I addressed your comments.

@jihoonson
Copy link
Copy Markdown
Contributor Author

@capistrant do you have more comments?

Copy link
Copy Markdown
Contributor

@capistrant capistrant left a comment

Choose a reason for hiding this comment

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

LGTM

@jihoonson
Copy link
Copy Markdown
Contributor Author

@capistrant thanks for the review.

@jihoonson jihoonson merged commit 16acd66 into apache:master Mar 5, 2021
@clintropolis clintropolis added this to the 0.22.0 milestone Aug 12, 2021
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