fix: Add sqlparse dependency - #171
c24t merged 3 commits into
Conversation
Sorry, something went wrong.
mf2199
left a comment
There was a problem hiding this comment.
👍 Good catch! This is something I wasn't sure about adding on a global scale.
Sorry, something went wrong.
| "proto-plus==1.11.0", | ||
| "libcst >= 0.2.5", | ||
| "proto-plus == 1.11.0", | ||
| "sqlparse >= 0.3.0", |
There was a problem hiding this comment.
sqlparse is only used for dbapi and is not required for using the Cloud Spanner client. I think it makes more sense to have this as a separate extra dependency similar to the OpenTelemetry dependencies under "tracing".
Sorry, something went wrong.
There was a problem hiding this comment.
The problem with making this an optional dependency is that we've got code in the library that will fail to import without it. The library still works without tracing, it just doesn't emit traces.
If we want to avoid sqlparse as a required dependency I see two options: (1) make it an optional import and catch the import errors in the DBAPI package and log an error, or (2) make spanner_dbapi a separate installable package in this repo, with its own set of requirements.
Sorry, something went wrong.
There was a problem hiding this comment.
As discussed I think (2) is the best option
Sorry, something went wrong.
larkee
left a comment
There was a problem hiding this comment.
LGTM. I would prefer to have spanner_dbapi as a separate installable before doing a release but I won't block you on this.
Sorry, something went wrong.
| "proto-plus==1.11.0", | ||
| "libcst >= 0.2.5", | ||
| "proto-plus == 1.11.0", | ||
| "sqlparse >= 0.3.0", |
There was a problem hiding this comment.
As discussed I think (2) is the best option
Sorry, something went wrong.
🤖 I have created a release \*beep\* \*boop\* --- ## [2.1.0](https://www.github.com/googleapis/python-spanner/compare/v2.0.0...v2.1.0) (2020-11-24) ### Features * **dbapi:** add aborted transactions retry support ([#168](https://www.github.com/googleapis/python-spanner/issues/168)) ([d59d502](https://www.github.com/googleapis/python-spanner/commit/d59d502590f618c8b13920ae05ab11add78315b5)), closes [#34](https://www.github.com/googleapis/python-spanner/issues/34) [googleapis/python-spanner-django#544](https://www.github.com/googleapis/python-spanner-django/issues/544) * remove adding a dummy WHERE clause into UPDATE and DELETE statements ([#169](https://www.github.com/googleapis/python-spanner/issues/169)) ([7f4d478](https://www.github.com/googleapis/python-spanner/commit/7f4d478fd9812c965cdb185c52aa9a8c9e599bed)) ### Bug Fixes * Add sqlparse dependency ([#171](https://www.github.com/googleapis/python-spanner/issues/171)) ([e801a2e](https://www.github.com/googleapis/python-spanner/commit/e801a2e014fcff66a69cb9da83abedb218cda2ab)) ### Reverts * Revert "test: unskip list_backup_operations sample test (#170)" (#174) ([6053f4a](https://www.github.com/googleapis/python-spanner/commit/6053f4ab0fc647a9cfc181e16c246141483c2397)), closes [#170](https://www.github.com/googleapis/python-spanner/issues/170) [#174](https://www.github.com/googleapis/python-spanner/issues/174) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please).
#160 added sqlparse as a test dependency only, but included non-test code that imports it. Running the tests outside of nox gives this error: E ModuleNotFoundError: No module named 'sqlparse'.
This PR makes sqlparse a regular non-test dependency.
The version number comes from googleapis/python-spanner-django@2b096c5.