plausible / ecto_ch

Ecto ClickHouse adapter
MIT License
72 stars 10 forks source link

`mix ecto.load` does not work when a port is configured #107

Closed hkrutzer closed 1 year ago

hkrutzer commented 1 year ago

When explicitly configuring a port for the adapter, this port will also be used when running mix ecto.load.

However, as this command uses the command line client, the command line client will now try to connect over the HTTP port, which will fail.

ruslandoga commented 1 year ago

Oh, right. I'm not sure what we should do in that case. Should we rewrite 8123 to 9000? Or should we print a warning?

hkrutzer commented 1 year ago

I don't think the configured port can ever be used, as it should always be the HTTP(S) port. But there's also no way to configure the port as a parameter of the ecto.load command, so if you want to use e.g. port 9001, it will have to have that value only for ecto.load, using an ENV var or something..

Perhaps the following is best:

ruslandoga commented 1 year ago

I think we can rewrite ecto.load to use our http client (Ch) instead of clickhouse-client binary. I was initially against that idea since Ecto.Adapters.Postgres used psql and pg_dump but it doesn't seem like using clickhouse-client would work for us.

hkrutzer commented 1 year ago

That also seems like a good option. It seems all the adapters that have a dump/load function use the native client (e.g. SQLite3 also does this), but I'm not sure why.

ruslandoga commented 1 year ago

@hkrutzer 👋

I've started a PR https://github.com/plausible/ecto_ch/pull/111 for structure_load over http, could you please try it out to see if the issue with custom port still exists?

hkrutzer commented 1 year ago

Works fine for me!