Skip to content

Conversation

@dimaslv
Copy link

@dimaslv dimaslv commented Feb 2, 2018

These commits allowed me to set up latest superset with our company's Druid v0.6 cluster. I have tried not to break anything for any newer versions.
This is second try for those changes to merge into upstream, first pull request was here #2331.

@xrmx
Copy link
Contributor

xrmx commented Feb 2, 2018

Just a couple of thoughts:

  • any chance that pydruid sqlalchemy dialect already handles that?
  • passing the version as parameter on an object does not look that nice, either storing the version as an attribute in the object or creating a subclass for older apis may look better

@mistercrunch
Copy link
Member

@xrmx doesn't have SQL support at all. I think SQLAlchemy access to druid might require 0.10 or 0.11+ Druid backend

Copy link
Member

@mistercrunch mistercrunch left a comment

Choose a reason for hiding this comment

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

Can we just reference self.cluster.druid_version instead of passing it around?

col_obj.count_distinct = True
# Allow sum/min/max for long or double
if datatype == 'LONG' or datatype == 'DOUBLE':
if datatype == 'LONG' or datatype == 'DOUBLE' or datatype == 'FLOAT':
Copy link
Member

Choose a reason for hiding this comment

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

let's use datetype in ('foo', 'bar')

).format(obj=self)
return json.loads(requests.get(endpoint).text)['version']
ver = json.loads(requests.get(endpoint).text)['version']
if ver is None:
Copy link
Member

Choose a reason for hiding this comment

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

return ver or "0" instead of the 4 lines

@kristw kristw added enhancement:request Enhancement request submitted by anyone from the community inactive Inactive for >= 30 days .database labels Mar 20, 2019
@kristw kristw closed this Mar 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement:request Enhancement request submitted by anyone from the community inactive Inactive for >= 30 days

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants