Just some basic fixes I did just to run queries on both ODBC and REST#96
Just some basic fixes I did just to run queries on both ODBC and REST#963runkenzie wants to merge 1 commit intomarkhoerth:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #96 +/- ##
==========================================
- Coverage 52.98% 52.94% -0.04%
==========================================
Files 24 24
Lines 1576 1577 +1
==========================================
Hits 835 835
- Misses 741 742 +1
Continue to review full report at Codecov.
|
| 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 |
There was a problem hiding this comment.
please leave this. Its a passthrough to get to the lower level api.
There was a problem hiding this comment.
@3runkenzie was this still working in your tests without this removed?
There was a problem hiding this comment.
Yes. I realized that, but I removed it because if I left it there, I couldn't get it to work.
| flight: | ||
| port: 47470 | ||
| rest: | ||
| port: 7183 |
There was a problem hiding this comment.
this should be 9047 as a default
| def query( | ||
| token, | ||
| base_url, | ||
| rest_url, |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
|
|
||
| _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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
@rymurr the driver can not be found if this is not changed
There was a problem hiding this comment.
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.
3 participants
These are just some issues/problems I found that I made quick fixes for (not PR worthy). These changes are expedient in nature and I didn't want to try to change the whole layout of the code base so I made some naive fixes.