Sync with upstream 3.28.0 #364

dkropachev · 2024-08-09T13:27:23Z

it is a peace of #329 that updates master to 3.28.0

fruch · 2024-08-10T21:31:56Z

docs/index.rst

@@ -56,6 +56,9 @@ Contents
 :doc:`scylla-cloud-serverless`
    Connect to ScyllaDB Cloud Serverless

+:doc:`column-encryption`


We should consider if we want to support this feature or not

why not, it is completely client-side feature

more so, i think it makes sense to have an epic to bring it into other drivers

exactly that, it's not a feature we planned nor tested.

if we do plan and test, that's great, I would document it and "promate" it, before that.

It is being tested at tests/integration/standard/column_encryption/test_policies.py and I don't think there is anything that could trigger scylla-specific issue in this code, it encrypts original content, and serialize it to the blob and does reverse on unserialization.

Sure, let's invoke @roydahan and @mykaul. For context:
We are syncing python-driver with the upstream and in 3.27.0 datastax released client-isde column encryption, so we are getting it in our driver.

Core idea of this feature is to seamlessly encrypt and decrypt a blob on client side.
Along side with this feature they have integration tests with it which we are running.
My proposal is to include this feature into our documentation right away, reasoning is:

It is entierly client-side feature, and chance of getting scylla-specific bug is close to 0.

It is tested at integration suite.

Performance degradation is expected and stays at users discretion.

Yes, we've seen this feature.
Let's start with an Epic for "all drivers", there we can discuss and decide if we want it and if needed for all drivers.

Issue is here, so, should we remove it from the documentation for now?

Maybe we should write it as experimental for now.

I have temporary disabled it and created an issue to take a look into it.

fruch

LGTM

dkropachev · 2024-08-15T11:23:34Z

@Lorak-mmk , @roydahan , is it good to go?

Lorak-mmk · 2024-08-15T12:23:19Z

@Lorak-mmk , @roydahan , is it good to go?

I'm reviewing right now

Lorak-mmk

Commits that add column encryption policy make a lot of changes.
I think it would be better to drop those commits altogether until we decide if and how we want to implement the encryption - adding code later is much easier than removing it.

After reviewing this I remember why I dislike current process so much...

Lorak-mmk · 2024-08-15T12:24:02Z

docs/.nav

+column_encryption
+geo_types
+graph
+classic_graph
+graph_fluent
+CHANGELOG


I think wee should either remove changelog from here, or start creating our own changelog and put it here.

We also should remove graph and geo whatever, scylla doesn't support those.

And we cleared the references to them from docs

Lorak-mmk · 2024-08-15T12:25:59Z

.travis.yml

I don't think we need this file. Could you remove it? Or even better - drop the commit that adds it.

There are couple more files we would like to drop, I have included them into single commit and added them to .gitignore

One PR should focus on one thing: let's only merge upstream changes in this one.
I actually started working on a PR that removes unnecessary files and updates some other legacy stuff.

Agree, I though it is going to be small thing, but it had grown very quickly, moved out to a separate PR.

As of now, let's leave it in, and address all of the removals in one blow.

Lorak-mmk · 2024-08-15T14:56:56Z

setup.py

+    dependencies = [
+        'geomet>=0.1,<0.3',
+        'pyyaml > 5.0',
+        'six >=1.9',
+    ]


You are adding six to dependencies here, why? Myself and upstream worked hard to remove it, and I don't see it used anywhere.

Keep in mind it's an old version we are syncing, before your changes were merged

But yes we should drop it

Lorak-mmk · 2024-08-15T15:01:32Z

cassandra/cqltypes.py

    try:
        return parse_casstype_args(casstype)
    except (ValueError, AssertionError, IndexError) as e:
+        log.debug("Exception in parse_casstype_args: %s" % e)
        raise ValueError("Don't know how to parse type string %r: %s" % (casstype, e))




What is the reason for adding this debug log?

It was part of datastax#1161, removed.

Lorak-mmk · 2024-08-15T15:06:11Z

cassandra/policies.py

+            unix_socket_path = getattr(h, "_unix_socket_path", None)
+            if unix_socket_path:
+                self._allowed_hosts_resolved.append(unix_socket_path)


I don't understand why this change is necessary or what it does. Please explain it.

It creates dependancy cycle after cryptography PR.

Lorak-mmk · 2024-08-15T15:08:43Z

docs/.nav

+column-encryption
+geo_types
+graph
+classic_graph
+graph_fluent
+CHANGELOG
 faq


Not sure if my previous comment was saved or not, so I'll repeat it.
Linking upstream changelog in our docs will imo be confusing for our users.
We should either write our own changelog, or remove this from docs.
Given that this PR is about merging upstream, I think it should remove changelog from nav.

fruch · 2024-08-15T15:38:03Z

tox.ini

@@ -1,5 +1,5 @@
 [tox]


If we are on a spree of dropping files, I don't think any of us are using this one, and it doesn't seem to support all the supported python versions

Let's merge updates first, and then drop files. I mentioned one in my review, because it is added in this PR.

I have created a separate PR for it.

dkropachev · 2024-08-16T00:19:00Z

Commits that add column encryption policy make a lot of changes. I think it would be better to drop those commits altogether until we decide if and how we want to implement the encryption - adding code later is much easier than removing it.

After reviewing this I remember why I dislike current process so much...

CLE is highly integrated into cluster, session and even query, removing these commits will create unnecessary conflicts with any code done on top of it, also as bonus you are going to get a headache of merging them back and re-resolving same conflicts other way around when/if you decide to get this code in.

…atastax#1150)

* update RH nav order * add line break * add api

Added error handling blog reference.

This reverts commit 6894b02.

…1164)

…l encryption (datastax#1160)

…ion-specific CLE policies (datastax#1165)

…tastax#1161)

There was some seriouse issues in policy implementation in `3.27.0`. We want to inspect the feature before making it available.

dkropachev · 2024-11-10T14:49:39Z

Commits that add column encryption policy make a lot of changes. I think it would be better to drop those commits altogether until we decide if and how we want to implement the encryption - adding code later is much easier than removing it.

After reviewing this I remember why I dislike current process so much...

Since we have already removed docs, I can't tell you that leaving code in creates way less conflicts and makes merging so much easier.
Also we need to consider that we are not merging code that was not reviewed, it already went though review process on the upstream, in some cases, when we changed core on our side it does not mean much, but on the side of the code we did not touch it does.

dkropachev force-pushed the dk/sync-with-upstream-3.28.0 branch 4 times, most recently from 76e1a2f to 8cc41a4 Compare August 9, 2024 22:42

dkropachev marked this pull request as ready for review August 9, 2024 22:45

dkropachev force-pushed the dk/sync-with-upstream-3.28.0 branch 2 times, most recently from 81b3b33 to e344b51 Compare August 9, 2024 23:57

dkropachev requested review from Lorak-mmk, sylwiaszunejko and fruch August 9, 2024 23:58

dkropachev self-assigned this Aug 9, 2024

fruch reviewed Aug 10, 2024

View reviewed changes

dkropachev force-pushed the dk/sync-with-upstream-3.28.0 branch 4 times, most recently from 969c93b to 5357c34 Compare August 13, 2024 18:24

dkropachev requested review from fruch and roydahan August 13, 2024 18:37

fruch approved these changes Aug 13, 2024

View reviewed changes

Lorak-mmk requested changes Aug 15, 2024

View reviewed changes

fruch reviewed Aug 15, 2024

View reviewed changes

dkropachev force-pushed the dk/sync-with-upstream-3.28.0 branch 2 times, most recently from ad15d4d to acbf507 Compare August 15, 2024 23:57

dkropachev requested a review from Lorak-mmk August 16, 2024 00:24

Lorak-mmk mentioned this pull request Nov 6, 2024

Updates for documentation and some cleanups #371

Merged

absurdfarce and others added 2 commits November 10, 2024 10:37

Fix typo

71cb95b

PYTHON-1341 Impl of client-side column-level encryption/decryption (d…

3c47f35

…atastax#1150)

absurdfarce and others added 19 commits November 10, 2024 10:39

Release 3.27: changelog & version

5a4e660

update RH nav order (datastax#1154)

124a362

* update RH nav order * add line break * add api

remove future plans (datastax#1155)

52720de

Missed dependency on cryptography in setup.py

1752789

DOC-2813 (datastax#1145)

4c593ce

Added error handling blog reference.

DOC-3278 Update comment for retry policy (datastax#1158)

460a045

Fix for rendering of code blocks in CLE documentation (datastax#1159)

891bc21

remove unnecessary import __future__ (datastax#1156)

6c2891b

docs: convert print statement to function in docs (datastax#1157)

e8c8f5d

Revert "remove unnecessary import __future__ (datastax#1156)"

d751840

This reverts commit 6894b02.

PYTHON-1351 Convert cryptography to an optional dependency (datastax#…

dd1429f

…1164)

PYTHON-1350 Store IV along with encrypted text when using column-leve…

3066627

…l encryption (datastax#1160)

PYTHON-1356 Create session-specific protocol handlers to contain sess…

fa3754d

…ion-specific CLE policies (datastax#1165)

CONN-38 Notes for 3.27.0 on PYTHON-1350 (datastax#1166)

dbc4997

PYTHON-1352 Add vector type, codec + support for parsing CQL type (da…

a46907d

…tastax#1161)

Release 3.28.0: changelog & version

17696b7

Add .eggs to .gitignore

0860fd9

Remove underscore from column_encruption doc

3bc409c

Temporary disable AES256ColumnEncryptionPolicy

6ea0d65

There was some seriouse issues in policy implementation in `3.27.0`. We want to inspect the feature before making it available.

dkropachev force-pushed the dk/sync-with-upstream-3.28.0 branch from acbf507 to 6ea0d65 Compare November 10, 2024 14:44

Sync with upstream 3.28.0 #364

Are you sure you want to change the base?

Sync with upstream 3.28.0 #364

Conversation

dkropachev commented Aug 9, 2024 • edited Loading

Choose a reason for hiding this comment

dkropachev Aug 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dkropachev Aug 11, 2024 • edited Loading

Choose a reason for hiding this comment

dkropachev Aug 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dkropachev Aug 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fruch left a comment

Choose a reason for hiding this comment

dkropachev commented Aug 15, 2024

Lorak-mmk commented Aug 15, 2024

Lorak-mmk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dkropachev commented Aug 16, 2024

dkropachev commented Nov 10, 2024 • edited Loading

dkropachev commented Aug 9, 2024 •

edited

Loading

dkropachev Aug 10, 2024 •

edited

Loading

dkropachev Aug 11, 2024 •

edited

Loading

dkropachev Aug 11, 2024 •

edited

Loading

dkropachev Aug 11, 2024 •

edited

Loading

dkropachev commented Nov 10, 2024 •

edited

Loading