-
Notifications
You must be signed in to change notification settings - Fork 544
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PYTHON-1351 Convert cryptography to an optional dependency #1164
Conversation
…nce of cryptography
…, I'm looking at you)
I've (manually) confirmed that after this PR the tests in this package fail due to an inability to find imports in cassandra.column_encryption.policies if cryptography is NOT present. This result is entirely expected since the absence of cryptography results in cassandra.column_encryption._policies never being imported. Tests pass cleanly if cryptography is present. This is the same behaviour we see from the gremlin_python dependency in the fluent graph tests, which seems like a goodness since we were deliberately trying to model the implementation here after what was done with gremlin_python. This isn't an issue for the Jenkins builds since cryptography is included in the default environment there due to the inclusion of test-datastax-requirements.txt in that environment. It would've been included anyway as a transitive dependency of Twisted but that's a different story. |
…n TravisCI builds. This keeps Travis closer in alignment to what's being done on Jenkins.
@jgillenwater I included you here for review of the documentation changes associated with this change. I was debating adding more detail to the column_encryption.rst page about how the cryptography module was no longer required... but the more I thought about it the more it seemed like it duplicated what's added in installation.rst... and that definitely seems like the right place for it. If you need to re-assign this please feel free to do so. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor docs edits.
Co-authored-by: Jamie Gillenwater <jamie.gillenwater@datastax.com>
want to use the CLE feature with a version of the driver after 3.27.0 you will need | ||
to take additional steps to install the cryptography module. | ||
|
||
You can install this module along with the driver by specifying the `cle` extra:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"extras" is closer to the actual Python terminology than "requirements" so I changed it here and above
docs/installation.rst
Outdated
automatically downloaded and installed when that version of the driver is installed. | ||
Later versions of the driver do not specify this module as a requirement, so if you | ||
want to use the CLE feature with a version of the driver after 3.27.0 you will need | ||
to take additional steps to install the cryptography module. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could see this change being a source of confusion so I wanted to very explicitly spell out the differences between the versions. It's a bit more verbose but I'd argue this is a case where we want to provide a bit more information to make the constraints clear to our users.
pip install cryptography | ||
|
||
Any version of cryptography >= 35.0 will work for the CLE feature. You can find additional | ||
details at `PYTHON-1351 <https://datastax-oss.atlassian.net/browse/PYTHON-1351>`_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant to include the link to the relevant JIRA ticket in my initial version and just forgot it. We don't have any other links to PYTHON tickets in this doc but it seemed like a good idea for this case which (as discussed above) is a bit more complicated than a normal doc change.
@jgillenwater My doc changes are in. Take a look when you have time and let me know what you think! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docs edits
Co-authored-by: Jamie Gillenwater <jamie.gillenwater@datastax.com>
Latest doc changes reflect the outcome of a conversation between @jgillenwater and myself. Calling it good with what we have now. |
Note: appveyor reports a warning due to missing init file: package init file 'cassandra\column_encryption_init_.py' not found (or not a regular file) |
…sync_with_upstream_3.29.1 version 3.28.0 * tag '3.28.0' of https://github.com/datastax/python-driver: Release 3.28.0: changelog & version PYTHON-1352 Add vector type, codec + support for parsing CQL type (datastax#1161) Update docs.yaml to point to most recent 3.27.0 docs changes CONN-38 Notes for 3.27.0 on PYTHON-1350 (datastax#1166) PYTHON-1356 Create session-specific protocol handlers to contain session-specific CLE policies (datastax#1165) PYTHON-1350 Store IV along with encrypted text when using column-level encryption (datastax#1160) PYTHON-1351 Convert cryptography to an optional dependency (datastax#1164) Jenkinsfile cleanup (datastax#1163) PYTHON-1343 Use Cython for smoke builds (datastax#1162) Don't fail when inserting UDTs with prepared queries with some missing fields (datastax#1151) Revert "remove unnecessary import __future__ (datastax#1156)" docs: convert print statement to function in docs (datastax#1157) remove unnecessary import __future__ (datastax#1156) Update docs.yaml to include recent fixes to CLE docs Fix for rendering of code blocks in CLE documentation (datastax#1159) DOC-3278 Update comment for retry policy (datastax#1158) DOC-2813 (datastax#1145) Remove different build matrix selection for develop branches (datastax#1138)
No description provided.