search

scylladb / python-driver

ScyllaDB Python Driver, originally DataStax Python Driver for Apache Cassandra
https://python-driver.docs.scylladb.com
Apache License 2.0
70 stars 42 forks source link

Sync with upstream up to 3.29.1 #329

Open fruch opened 2 months ago

fruch commented 2 months ago

doing it slow one version at a time

fruch commented 2 months ago

All merges are done,

but need to fix the fallout, mostly one ugly circular dependency issue between our UnixSocket recent implementation to newly introduce change related to column encryption feature...

@Lorak-mmk @sylwiaszunejko, one of want to pick that up ? (feel free to take on over this PR/branch, or clone it into a new one, and look into it

Lorak-mmk commented 2 months ago

One thing to consider: my PR that removed six dependency was slightly different in the upstream (I remember that I had to do some changes before opening it to upstream). We should check what are the differences and modify the code accordingly.

fruch commented 2 months ago

One thing to consider: my PR that removed six dependency was slightly different in the upstream (I remember that I had to do some changes before opening it to upstream). We should check what are the differences and modify the code accordingly.

Not sure how critical it is, but you can compare rebase the PR based on this branch and check the differences.

I might say that the sync is easier than I thought it would be (saying this before attending all of the fallout ...)

The process works quite well (i.e. the scripts for doing the sync and CI is quite extensive), in compresente to cqlsh which didn't yet synced, and is much more complex cause it's part of a repo, so you can't directly do merges...

Lorak-mmk commented 2 months ago

Eh, I really dislike that we have so many forks, it makes development so much harder...

fruch commented 2 months ago

Eh, I really dislike that we have so many forks, it makes development so much harder...

other option is to write it all from scratch, not sure it's much better. anyhow until then, we need to keep maintaining this one, for our users (and for ourselves)

fruch commented 2 months ago

@Lorak-mmk we need to take a call here, on 3.27.0 they introduce client side Column Encryption feature.

Meanwhile I don't mention it on our docs, and we should consider if to disable it in our fork (at least until we review/test it and gives it a thumbs up)

I don't think we've seen new feature much since we forked...

@roydahan @tzach, your thoughts about how to handle this feature ?

roydahan commented 2 months ago

Is it a client side only feature?

fruch commented 2 months ago

Is it a client side only feature?

yes

fruch commented 2 months ago

@Lorak-mmk

Took me a while to get the windows wheel building in shape it was failing in testing with python 3.12, cause of missing libev (i.e. no fallback to asyncio anymore) I've figured out a way to retrieve a compiled version of libev, but only for win64, not with win32 I'm giving up on wheels for win32, i.e. wheels we can't cover with tests are not really someone we can consider supported

(we have more like that, all the PyPy ones, but that's a fight for another day)

the only pending issue here, is getting agreement on how to treat Column Encryption feature. and get it reviewed

fruch commented 2 months ago

One more issue I remember we might have https://github.com/scylladb/scylla-cqlsh/issues/90

cqlsh would break with python 3.12

Lorak-mmk commented 1 month ago

That's a lot of code to review... I'll try to get to it soon. Do you think it would be possible to create an automation to open a PR to our fork whenever a PR is merged to upstream (or non-PR commits are added to master in upstream)? That way we could review the changes as they come instead of dealing with those insane massive diffs.

fruch commented 1 month ago

That's a lot of code to review... I'll try to get to it soon. Do you think it would be possible to create an automation to open a PR to our fork whenever a PR is merged to upstream (or non-PR commits are added to master in upstream)? That way we could review the changes as they come instead of dealing with those insane massive diffs.

We might consider that, but it would be a bit complicated, and we'll need to make sure we merge them in the right order.

It might be easy if we do it per release, in this PR we have 3 releases at the same time.

Lorak-mmk commented 1 month ago

@Lorak-mmk we need to take a call here, on 3.27.0 they introduce client side Column Encryption feature.

Meanwhile I don't mention it on our docs, and we should consider if to disable it in our fork (at least until we review/test it and gives it a thumbs up)

I don't think we've seen new feature much since we forked...

@roydahan @tzach, your thoughts about how to handle this feature ?

Are there tests for it?