Add basic query feature #1
Conversation
It accepts all formats and it integrates with Explorer if the `:explorer` format is used.
|
This PR also don't tackle the CI workflow. I should create that infra in another PR as well. |
| @@ -1,12 +1,167 @@ | |||
| defmodule ReqCh do | |||
| @moduledoc """ | |||
| A Req plugin for ClickHouse. | |||
|
|
|||
| By default, `ReqCh` will use TSV as the default output format. | |||
There was a problem hiding this comment.
nitpick: I'd would call it ReqCH, we shorten GitHub to GH so we should shorten ClickHouse to CH.
| iex> req = Req.new() |> ReqCh.attach() | ||
| iex> Req.post!(req, clickhouse: "SELECT number from system.numbers LIMIT 3").body | ||
| "0\\n1\\n2\\n" |
There was a problem hiding this comment.
Yeah so similar to ReqAthena discussion, this really feels like better achieved with:
iex> ReqCH.query!("SELECT ...")
iex> req = ReqCH.new(hostname: "clickhouse.cloud", retry: :transient)
iex> ReqCH.query!(req, "SELECT ...", retry: false)Below is intentionally bizarre and unfair to this plugin because it is true for any plugin, but like, isn't it weird we can make this call?
req = Req.new() |> ReqCh.attach()
Req.get!(req, url: "https://httpbin.org", scheme: "http")Just to be clear, this is totally on me FWIW.
| url_parts = [ | ||
| get_option(request, :scheme, "http"), | ||
| "://", | ||
| maybe_credentials(request), |
There was a problem hiding this comment.
instead of sending the credentials in the URL (which could show up in logs and stuff) I wonder if we can use the auth: {:basic, "user:pass"} feature.
| Req.Request.halt(request, %{ | ||
| response | ||
| | body: Explorer.DataFrame.load_parquet!(response.body) | ||
| }) |
There was a problem hiding this comment.
could also be:
Req.Request.halt(request, update_in(response.body, &Explorer.DataFrame.load_parquet!/1))
* Apply some suggestions from Wojtek #1 (review) * ReqCh -> ReqCH * Add missing ReqCH.attach/1 * Update lib/req_ch.ex Co-authored-by: Wojtek Mach <[email protected]> --------- Co-authored-by: Wojtek Mach <[email protected]>
This still don't support params - I should tackle that in another PR.