druid-io / pydruid

A Python connector for Druid
Other
507 stars 198 forks source link

Asynchronous client is not working #63

Closed mghosh4 closed 7 years ago

mghosh4 commented 7 years ago

I tried running the pydruid async client. It does not work. The issue is that it uses AsyncHttpClient from tornado which is the interface. Instead we should be using one of the implementation, SimpleHttpAsyncClient or CurlAsyncHTTPClient. The latter is better as described in this link http://www.tornadoweb.org/en/stable/httpclient.html#module-tornado.simple_httpclient. I have made this one line fix and can create a patch if it is fine by everyone.

mghosh4 commented 7 years ago

Using the curl based client will put 2 additional dependencies. You need to install libcurl4-openssl-dev in linux and also pycurl python package. Should I just change the doc for this? Or is there a way to add this dependency in the patch?

turu commented 7 years ago

Are you sure it doesn't work? I'm asking because it does work on production since many months in our stack. You may want to take a look at this: http://www.tornadoweb.org/en/stable/httpclient.html#tornado.httpclient.AsyncHTTPClient ;)

mghosh4 commented 7 years ago

I see. I might be using the constructor wrong then. From what I understand if I configure AsyncHTTPClient to use the correct class. It should work? Let me know if I do not understand this.

turu commented 7 years ago

@mghosh4 You just need to use the constructor as described in the linked docs. The simple_httpclient is fully sufficient and definitely should stay as the default, since it doesn't require any additional dependencies installed on your machine - unlike the curl_httpclient.

Perhaps, a patch allowing people to override the default client could be useful though. Feel free to submit one.

turu commented 7 years ago

@gianm @xvrl @fjy I would suggest closing this issue, since its name may be misleading people to think that the async client is broken - It isn't.

fjy commented 7 years ago

Thanks @turu