Merging both connectors? about clickhouse-sqlalchemy HOT 4 CLOSED

Hi! I was inspired by your connector when developing mine. Your dialect is pretty nice too, but there were some troubles with infi.clickhouse_orm integration, per row INSERTs, lack of declarative meta support and so on. That's why I've decided to make my own connector instead of making many PRs into your dialect and changing whole approach for SQL-generation.

But there is still a lot of work to do. Especially in HTTP connector. I'll appreciate any help and PRs!

I need some time to check what parts from your dialect can be merged with mine.

from clickhouse-sqlalchemy.

Hi again!

I've take a brief look at current version of your connector.

I assume you wanted to merge my dialect into yours. But in the current way of things it's easier to merge your connector into mine, unfortunately. And here is why:

• I haven't find fixes than can be merged into mine connector. At least now.
• HTTP transport dropping is in my short-time plans. It'll help to concentrate on dialect internals, instead of parsing and dealing with tricky escaping. Just imagine how painful parsing of Array(Array(String)) is 😞. Switching to native transport is really a great move forward.
• infi.clickhouse_orm is not really required for performing queries.

In this way of merge my codebase will replace the most of yours. So, it's kind of an ideological point.
Another choice is not to merge at all as we are using different approaches to performing queries.

What's your opinion? Yup, I realize that your project has more stars and installs.

Here is short story how we ended up with ours connector:

We were need production-ready dialect that satisfies the requirements of our project within two weeks. I've find your dialect but it seemed to be abandoned for 3 months after initial commit. Integration with infi was also broken. And unfortunately existing dialect didn't meet all the requirements. So we quickly build ours and then publish it on github later.

from clickhouse-sqlalchemy.

Hey, sorry for the late reply, I just got back from vacation. Personally I don't mind merging either way, one of the things I'd like to retain though is the HTTP transport as it's the only one that supports encryption. I can fork it though if you don't want to continue maintaining it. Other than that, my use case is just making simple SELECT queries, so anything fits.

from clickhouse-sqlalchemy.

I hope you had great vacation.

HTTP/HTTPS transport can retain if you can and will continue to support it. If you don't have time for it I'd appreciate any PR and bugfixes, but default transport should be the native one after all. If HTTP transport retains, we should get rid of infi.clickhouse_orm. One ORM inside another is not good idea. Only parsing and escaping is required, pure requests package is good enough as transport.

Do you agree with my points?

Anyway, feel free to fork.

from clickhouse-sqlalchemy.