Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: missing method implementation and potential bug in the DatabaseWrapper class #545

Merged
merged 8 commits into from Dec 17, 2020
Merged

fix: missing method implementation and potential bug in the DatabaseWrapper class #545

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
35d2e68
fix: implementation of `init_connection_state()` + potential bug in `…
mf2199 Oct 25, 2020
6496a18
Merge branch 'master' into django-spanner-base
mf2199 Oct 30, 2020
5a8ac4e
chore: implementing suggested changes
mf2199 Oct 30, 2020
83f6217
Merge branch 'master' into django-spanner-base
mf2199 Nov 11, 2020
0faab71
test: unit test coverage for `base.py`
mf2199 Nov 15, 2020
14ebac9
chore: [temporarily] skipping Python-3.5
mf2199 Nov 15, 2020
35b207e
Use django 2.2 in nox tests
c24t Dec 16, 2020
29652ac
Merge branch 'master' into django-spanner-base
c24t Dec 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 13 additions & 7 deletions django_spanner/base.py
View file
Open in desktop
Expand Up @@ -5,7 +5,9 @@
# https://developers.google.com/open-source/licenses/bsd

from django.db.backends.base.base import BaseDatabaseWrapper
from google.cloud import spanner_dbapi as Database, spanner_v1 as spanner

import google.cloud.spanner_dbapi as Database
c24t marked this conversation as resolved.
Show resolved Hide resolved
import google.cloud.spanner_v1 as spanner

from .client import DatabaseClient
from .creation import DatabaseCreation
Expand Down Expand Up @@ -81,8 +83,9 @@ class DatabaseWrapper(BaseDatabaseWrapper):
# special characters for REGEXP_CONTAINS operators (e.g. \, *, _) must be
# escaped on database side.
pattern_esc = r'REPLACE(REPLACE(REPLACE({}, "\\", "\\\\"), "%%", r"\%%"), "_", r"\_")'
# These are all no-ops in favor of using REGEXP_CONTAINS in the customized
# lookups.

# These are all no-ops in favor of using REGEXP_CONTAINS
# in the customized lookups.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# These are all no-ops in favor of using REGEXP_CONTAINS
# in the customized lookups.
# These are all no-ops in favor of using REGEXP_CONTAINS in the customized
# lookups.

Why rewrap this?

pattern_ops = {
"contains": "",
"icontains": "",
Expand Down Expand Up @@ -124,7 +127,9 @@ def get_new_connection(self, conn_params):
return Database.connect(**conn_params)

def init_connection_state(self):
pass
self.connection.close()
c24t marked this conversation as resolved.
Show resolved Hide resolved
database = self.connection.database
self.connection.__init__(self.instance, database)

def create_cursor(self, name=None):
return self.connection.cursor()
Expand All @@ -134,12 +139,13 @@ def _set_autocommit(self, autocommit):
self.connection.autocommit = autocommit

def is_usable(self):
if self.connection is None:
if self.connection is None or self.connection.is_closed:
return False

try:
# Use a cursor directly, bypassing Django's utilities.
self.connection.cursor().execute("SELECT 1")
except Database.Error:
return False
else:
return True

return True