Skip to content
This repository was archived by the owner on Mar 18, 2022. It is now read-only.

Just some basic fixes I did just to run queries on both ODBC and REST #96

Open
3runkenzie wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Just some basic fixes I did just to run queries on both ODBC and REST #96

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions dremio_client/conf/config_default.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,5 @@ odbc:
port: 31010
flight:
port: 47470
rest:
port: 7183

@rymurr rymurr Jul 2, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this should be 9047 as a default

14 changes: 11 additions & 3 deletions dremio_client/dremio_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,17 +65,24 @@ def __init__(self, config):
)
self._flight_port = config["flight"]["port"].get(int)
self._odbc_port = config["odbc"]["port"].get(int)
self._rest_port = config["rest"]["port"].get(int)

self._username = config["auth"]["username"].get()
self._password = config["auth"]["password"].get()
self._token = auth(self._base_url, config)
self._rest_url = (
("https" if config["ssl"].get(bool) else "http")
+ "://"
+ self._hostname
+ (":{}".format(self._rest_port) if port else "")
)
self._token = auth(self._rest_url, config)
self._ssl_verify = config["verify"].get(bool)
self._catalog = catalog(self._token, self._base_url, self.query, self._ssl_verify)
self._catalog = catalog(self._token, self._rest_url, self.query, self._ssl_verify)
self._reflections = list()
self._wlm_queues = list()
self._wlm_rules = list()
self._votes = list()
self._simple = SimpleClient(config)
#self._simple = SimpleClient(config) # took this off because I didn't know how it was intended to be used for now or in the future

@rymurr rymurr Jul 2, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

please leave this. Its a passthrough to get to the lower level api.

@be-rock be-rock Sep 21, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@3runkenzie was this still working in your tests without this removed?

@3runkenzie 3runkenzie Sep 25, 2020

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes. I realized that, but I removed it because if I left it there, I couldn't get it to work.


def simple(self):
return self._simple
Expand Down Expand Up @@ -132,6 +139,7 @@ def query(self, sql, pandas=True, method="flight"):
return query(
self._token,
self._base_url,
self._rest_url,
self._hostname,
self._odbc_port,
self._flight_port,
Expand Down
6 changes: 3 additions & 3 deletions dremio_client/odbc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@


_WINDOWS_DRIVER = "Dremio Connector"
_OSX_DRIVER = "Dremio ODBC Driver"
_OSX_DRIVER = "/Library/Dremio/ODBC/lib/libdrillodbc_sbu.dylib" # had to add this for the driver path

@rymurr rymurr Jul 2, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

what happens if this is not changed? I thought the purpose of this was to fix #66 so that odbc worked w/o having to give absolute paths?

@be-rock be-rock Sep 21, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@rymurr the driver can not be found if this is not changed

@3runkenzie 3runkenzie Sep 25, 2020

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes. You are right. I was doing that on my end because pyodbc.drivers() returned an empty list so I included the full path for the Driver to get it to work.

_LINUX32_DRIVER = "Dremio ODBC Driver 32-bit"
_LINUX64_DRIVER = "Dremio ODBC Driver 64-bit"
_DRIVER = None
Expand All @@ -41,9 +41,9 @@ def _get_driver_name():
_DRIVER = _LINUX64_DRIVER
else:
_DRIVER = _LINUX32_DRIVER
if "darwin" in sys.platform:
elif "darwin" in sys.platform: # lines 44 and 46 were not mutually exclusive so decided to change
_DRIVER = _OSX_DRIVER
if "win" in sys.platform:
elif "win" in sys.platform:
_DRIVER = _WINDOWS_DRIVER
logging.debug("Using %s as the odbc driver", _DRIVER)

Expand Down
3 changes: 2 additions & 1 deletion dremio_client/query.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
def query(
token,
base_url,
rest_url,

@rymurr rymurr Jul 2, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

rest_url takes the place of base_url so we can remove base_url here. But they are the same value so I don't understand the purpose?

@be-rock be-rock Sep 21, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@3runkenzie?

@3runkenzie 3runkenzie Sep 25, 2020

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes. I wanted to keep the parameters you had there initially (hence base_url does not get used) to incorporate a concept of having 2 types of urls.

self._rest_url = (
            ("https" if config["ssl"].get(bool) else "http")
            + "://"
            + self._hostname
            + (":{}".format(self._rest_port) if port else "")
        )

self._base_url = (
            ("https" if config["ssl"].get(bool) else "http")
            + "://"
            + self._hostname
            + (":{}".format(port) if port else "")
        )

since the ports are different for ODBC vs REST.

hostname,
odbc_port,
flight_port,
Expand All @@ -64,7 +65,7 @@ def query(
return _odbc_query(sql, hostname=hostname, port=odbc_port, username=username, password=password)
except Exception:
logging.warning("Unable to run query as odbc, downgrading to rest")
results = _rest_query(token, base_url, sql, ssl_verify=ssl_verify)
results = _rest_query(token, rest_url, sql, ssl_verify=ssl_verify)
if pandas and not NO_PANDAS:
return pd.DataFrame(results)
return list(results)