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 jdbc even if trailing / is missing #11737

Merged
merged 2 commits into from
Sep 29, 2021

Conversation

clintropolis
Copy link
Member

Description

Currently JDBC connections to Druid require that the trailing / be part of the request url so that it matches exactly, else it will not be handled by the avatica machinery. This isn't very user-friendly, this PR modifies the Avatica endpoint handlers to accept with or without the trailing /, e.g. /druid/v2/sql/avatica/ or /druid/v2/sql/avatica (and also protobuf).

This PR has:

  • been self-reviewed.
  • 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.

Copy link
Member

@FrankChen021 FrankChen021 left a comment

Choose a reason for hiding this comment

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

👍

@clintropolis
Copy link
Member Author

I marked this as WIP because seeing some strange flakiness on Avatica unit tests, https://app.travis-ci.com/github/apache/druid/jobs/539085044#L2192

I don't really understand yet how the changes could cause that, but I can replicate locally and also don't see this behavior outside of this branch.

@clintropolis
Copy link
Member Author

I don't really understand yet how the changes could cause that, but I can replicate locally and also don't see this behavior outside of this branch.

finally got back to this, the issue seems to have been caused by the test having a maximum connection limit of 3, but the newly added connection made there be 4

@clintropolis clintropolis removed the WIP label Sep 24, 2021
@clintropolis clintropolis merged commit 11017ef into apache:master Sep 29, 2021
@clintropolis clintropolis deleted the jdbc-trailing-slash branch September 29, 2021 20:59
@abhishekagarwal87 abhishekagarwal87 added this to the 0.23.0 milestone May 11, 2022
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.

3 participants