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

DB API - Adding support for dynamic parameters #258

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

harnasz
Copy link

@harnasz harnasz commented Apr 12, 2021

This PR adds support for Druids SQL Dynamic Parameters.

  • It's backward compatiable for the existing DB API and is a non breaking change and requires opt in
  • To opt into using Dynamic Parameters dynamic_parameters=True can be set on the db.api.connect method. It's set to False by default
  • It ensures correct ordering in thecontext.parameters JSON payload, regardless of the Dict parameters insertion order ensuring support for Python Verisons < 3.7
  • Fixes a breaking change introduced in fix: visit_ methods raise CompileError exception childs #243 for SQLAlchemy, see more below on this
  • Produced queries/requests been tested against Druid 0.18 and above.
  • Documented with a working example, inspired from the Druid Tutorial Documentation

SQLAlchemy breaking change fix
Exception was being raised when using the DB API independantly of SQLAlchemy, it was attempting to import CompileError but this is an optional dependency as defined in extras_require. This was introduced in the enhancement #243 - the fix facilitates backwards compatibility but preserves the new enhancement.

tomhpz and others added 4 commits April 11, 2021 18:33
Exception was being raised when using the DB API - independent of SQLAlchemy,
it was attempting to import `CompileError` but this is an optional
dependency as defined in `extras_require`.

This was introduced in the enhancement druid-io#243 - the fix facilitates backwards
compatibility but includes the new enhancement.
@harnasz harnasz marked this pull request as ready for review April 16, 2021 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants