Skip to content

Commit 947fa0b

Browse files
committed
refactor(sqlalchemy-spanner): remove redundant version checks from individual compliance/migration sessions now that system dispatcher handles them upfront
1 parent 2b056ab commit 947fa0b

10 files changed

Lines changed: 285 additions & 149 deletions

File tree

‎packages/sqlalchemy-spanner/create_test_config.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
# limitations under the License.
1616

1717
import configparser
18+
import os
1819
import sys
1920

2021

@@ -41,7 +42,8 @@ def set_test_config(
4142
config.add_section("db")
4243
config["db"]["default"] = url
4344

44-
with open("test.cfg", "w") as configfile:
45+
config_filename = os.getenv("SQLALCHEMY_SPANNER_CONFIG", "test.cfg")
46+
with open(config_filename, "w") as configfile:
4547
config.write(configfile)
4648

4749

‎packages/sqlalchemy-spanner/create_test_database.py‎

Lines changed: 50 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,18 @@
1515
# limitations under the License.
1616

1717
import os
18+
import pathlib
1819
import re
1920
import time
21+
import uuid
2022

23+
from create_test_config import set_test_config
2124
from google.api_core import datetime_helpers
2225
from google.api_core.exceptions import AlreadyExists, ResourceExhausted
2326
from google.cloud.spanner_v1 import Client
2427
from google.cloud.spanner_v1.database import Database
2528
from google.cloud.spanner_v1.instance import Instance
2629

27-
from create_test_config import set_test_config
28-
2930
USE_EMULATOR = os.getenv("SPANNER_EMULATOR_HOST") is not None
3031

3132
PROJECT = os.getenv(
@@ -55,8 +56,6 @@ def delete_stale_test_instances():
5556
create_time = int(instance.labels["created"])
5657
if create_time > cutoff:
5758
continue
58-
# Backups are not used in sqlalchemy dialect test,
59-
# therefore instance can just be deleted.
6059
try:
6160
instance.delete()
6261
time.sleep(5) # Sleep for 5 seconds to give time for cooldown.
@@ -69,26 +68,39 @@ def delete_stale_test_instances():
6968

7069

7170
def delete_stale_test_databases():
72-
"""Delete test databases that are older than 10 minutes.
73-
74-
In this test suite, active databases typically finish running in ~5 minutes.
75-
To prevent concurrent Kokoro runs from accidentally deleting each other's
76-
active databases we use a 10-minute safety threshold. Without an aggressive
77-
cutoff we quickly bump up against Cloud Spanner's limit of 100 databases per instance.
71+
"""Delete test databases that are older than 4 hours.
72+
73+
Uses a .stale_cleanup_done sentinel file gate to ensure this global sweep
74+
runs exactly once at the start of a test run across parallel/parametrized sessions,
75+
preventing concurrent sessions from interfering with each other.
7876
"""
79-
cutoff = (int(time.time()) - 10 * 60) * 1000
77+
marker = pathlib.Path(".stale_cleanup_done")
78+
if marker.exists():
79+
return
80+
81+
try:
82+
marker.touch(exist_ok=False)
83+
except FileExistsError:
84+
return # Another parallel process already performed cleanup
85+
86+
cutoff = (int(time.time()) - 4 * 60 * 60) * 1000
8087
instance = CLIENT.instance("sqlalchemy-dialect-test")
8188
if not instance.exists():
8289
return
8390
database_pbs = instance.list_databases()
8491
for database_pb in database_pbs:
8592
database = Database.from_pb(database_pb, instance)
86-
# Parse creation time from database ID first (e.g. "sqlalchemy-test-1779989493809")
87-
# to be 100% independent of emulator metadata or GCP Client API create_time gaps!
93+
94+
# Parse creation time from database ID first (e.g. "sp_test_1787069488_a3f2b1")
8895
create_time = None
89-
match = re.match(r"sqlalchemy-test-(\d+)", database.database_id)
96+
match = re.match(r"sp_test_(\d+)", database.database_id)
9097
if match:
91-
create_time = int(match.group(1))
98+
ts_str = match.group(1)
99+
ts_val = int(ts_str)
100+
if len(ts_str) == 10:
101+
create_time = ts_val * 1000
102+
else:
103+
create_time = ts_val
92104
elif database_pb.create_time is not None:
93105
create_time = datetime_helpers.to_milliseconds(database_pb.create_time)
94106

@@ -105,6 +117,12 @@ def delete_stale_test_databases():
105117

106118

107119
def create_test_instance():
120+
"""Create test instance and a session-isolated ephemeral test database.
121+
122+
To prevent cross-session test pollution and parallel execution race conditions,
123+
each test session generates a unique database ID combining a 10-digit timestamp
124+
and a 6-character random hex suffix (conforming to Spanner's regex: ^[a-z][a-z0-9_]{1,29}$).
125+
"""
108126
instance_id = "sqlalchemy-dialect-test"
109127
instance = CLIENT.instance(instance_id)
110128
if not instance.exists():
@@ -123,8 +141,12 @@ def create_test_instance():
123141
except AlreadyExists:
124142
pass # instance was already created
125143

126-
unique_resource_id = "%s%d" % ("-", 1000 * time.time())
127-
database_id = "sqlalchemy-test" + unique_resource_id
144+
# Generate a session-isolated unique database ID within Spanner 30-char limit
145+
# Format: sp_test_{timestamp_in_seconds}_{rand_hex6} (compliant with Spanner naming: ^[a-z][a-z0-9_]{1,29}$)
146+
creation_timestamp = time.time()
147+
timestamp_part = str(int(creation_timestamp))
148+
rand_part = uuid.uuid4().hex[:6]
149+
database_id = f"sp_test_{timestamp_part}_{rand_part}"
128150

129151
try:
130152
database = instance.database(database_id)
@@ -136,5 +158,14 @@ def create_test_instance():
136158
set_test_config(PROJECT, instance_id, database_id)
137159

138160

139-
delete_stale_test_databases()
140-
create_test_instance()
161+
def main(argv):
162+
config_filename = argv[0] if argv else os.getenv("SQLALCHEMY_SPANNER_CONFIG", "test.cfg")
163+
os.environ["SQLALCHEMY_SPANNER_CONFIG"] = config_filename
164+
165+
delete_stale_test_databases()
166+
create_test_instance()
167+
168+
169+
if __name__ == "__main__":
170+
import sys
171+
main(sys.argv[1:])

‎packages/sqlalchemy-spanner/drop_test_database.py‎

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,11 @@
1616

1717
import configparser
1818
import os
19+
import pathlib
1920
import re
20-
import time
21+
import sys
2122

22-
from google.api_core import datetime_helpers
23-
from google.api_core.exceptions import AlreadyExists, ResourceExhausted
2423
from google.cloud.spanner_v1 import Client
25-
from google.cloud.spanner_v1.database import Database
26-
from google.cloud.spanner_v1.instance import Instance
27-
28-
from create_test_config import set_test_config
2924

3025
USE_EMULATOR = os.getenv("SPANNER_EMULATOR_HOST") is not None
3126

@@ -43,21 +38,39 @@
4338
CLIENT = Client(project=PROJECT)
4439

4540

46-
def delete_test_database():
41+
def delete_test_database(config_filename="test.cfg"):
4742
"""Delete the currently configured test database."""
4843
config = configparser.ConfigParser()
49-
if os.path.exists("test.cfg"):
50-
config.read("test.cfg")
44+
config_path = pathlib.Path(config_filename)
45+
if config_path.exists():
46+
config.read(config_path)
5147
else:
5248
config.read("setup.cfg")
49+
5350
db_url = config.get("db", "default")
5451

5552
instance_id = re.findall(r"instances(.*?)databases", db_url)
5653
database_id = re.findall(r"databases(.*?)$", db_url)
5754

58-
instance = CLIENT.instance(instance_id="".join(instance_id).replace("/", ""))
59-
database = instance.database("".join(database_id).replace("/", ""))
55+
instance_id_str = "".join(instance_id).replace("/", "")
56+
database_id_str = "".join(database_id).replace("/", "")
57+
58+
instance = CLIENT.instance(instance_id=instance_id_str)
59+
database = instance.database(database_id_str)
6060
database.drop()
6161

62+
# Clean up session-specific config file
63+
if config_path.exists() and config_filename != "setup.cfg":
64+
try:
65+
config_path.unlink()
66+
except Exception:
67+
pass
68+
69+
70+
def main(argv):
71+
config_filename = argv[0] if argv else os.getenv("SQLALCHEMY_SPANNER_CONFIG", "test.cfg")
72+
delete_test_database(config_filename)
73+
6274

63-
delete_test_database()
75+
if __name__ == "__main__":
76+
main(sys.argv[1:])

0 commit comments

Comments
 (0)