FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Make option-like parameters (mostly `transaction`) keyword-only. · 7dracoder/python-spanner-orm@51cee69 · GitHub

/ python-spanner-orm Public
forked from google/python-spanner-orm

Commit 51cee69

Browse files
dseomn
committed
Make option-like parameters (mostly transaction) keyword-only.
Note that this change breaks the API, but I think it's worth it. In some cases, e.g., where(), this avoids needing to pass an unnamed `None` argument which I think is confusing. In the case of save_batch()'s `force_write` parameter, I think this can avoid potentially bad bugs. And in all cases, I think this makes the call sites more clear. Additionally: 1. Make the `transaction` parameter optional for all methods. 2. Fix some type annotations on those parameters.
1 parent de0016b commit 51cee69

7 files changed

Lines changed: 117 additions & 80 deletions

File tree

‎README.md‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -84,31 +84,32 @@ The two main ways of retrieving data through the ORM are ```where()``` and
8484
```find()```/```find_multi()```:
8585

8686
``` python
87-
# where() is invokes on a model class to retrieve models of that tyep. it takes a
88-
# transaction and then a sequence of conditions.
89-
# Most conditions that specify a Field, Index, Relationship, or Model can take
90-
# either the name of the object or the object itself
91-
test_objects = TestModel.where(None, spanner_orm.greater_than('value', '50'))
87+
# where() is invokes on a model class to retrieve models of that type. it takes
88+
# a sequence of conditions. Most conditions that specify a Field, Index,
89+
# Relationship, or Model can take either the name of the object or the object
90+
# itself
91+
test_objects = TestModel.where(spanner_orm.greater_than('value', '50'))
9292

9393
# To also retrieve related objects, the includes() condition should be used:
94-
test_and_other_objects = TestModel.where(None,
95-
spanner_orm.greater_than(TestModel.value, '50'),
96-
spanner_orm.includes(TestModel.fake_relationship))
94+
test_and_other_objects = TestModel.where(
95+
spanner_orm.greater_than(TestModel.value, '50'),
96+
spanner_orm.includes(TestModel.fake_relationship),
97+
)
9798

9899
# To create a transaction, run_read_only() or run_write() are used with the
99100
# method to be run inside the transaction and any arguments to passs to the method.
100101
# The method is invoked with the transaction as the first argument and then the
101102
# rest of the provided arguments:
102103
def callback_1(transaction, argument):
103-
return TestModel.find(transaction, id=argument)
104+
return TestModel.find(id=argument, transaction=transaction)
104105

105106
specific_object = spanner_orm.spanner_api().run_read_only(callback, 1)
106107

107108
# Alternatively, the transactional_read decorator can be used to clean up the
108109
# call a bit:
109110
@transactional_read
110111
def finder(argument, transaction=None):
111-
return TestModel.find(transaction, id=argument)
112+
return TestModel.find(id=argument, transaction=transaction)
112113
specific_object = finder(1)
113114
```
114115

@@ -131,7 +132,7 @@ models = []
131132
for i in range(10):
132133
key = 'test_{}'.format(i)
133134
models.append(TestModel({'key': key, 'value': value}))
134-
TestModel.save_batch(None, models)
135+
TestModel.save_batch(models)
135136
```
136137

137138
```spanner_orm.spanner_api().run_write()``` can be used for executing read-write

‎spanner_orm/admin/metadata.py‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,10 @@ def model(cls, table_name) -> Optional[Type[model.Model]]:
7171
def tables(cls) -> Dict[str, Dict[str, Any]]:
7272
"""Compiles table information from column schema."""
7373
column_data = collections.defaultdict(dict)
74-
columns = column.ColumnSchema.where(None,
75-
condition.equal_to('table_catalog', ''),
76-
condition.equal_to('table_schema', ''))
74+
columns = column.ColumnSchema.where(
75+
condition.equal_to('table_catalog', ''),
76+
condition.equal_to('table_schema', ''),
77+
)
7778
for column_row in columns:
7879
new_field = field.Field(
7980
column_row.field_type(), nullable=column_row.nullable())
@@ -82,9 +83,10 @@ def tables(cls) -> Dict[str, Dict[str, Any]]:
8283
column_data[column_row.table_name][column_row.column_name] = new_field
8384

8485
table_data = collections.defaultdict(dict)
85-
tables = table.TableSchema.where(None,
86-
condition.equal_to('table_catalog', ''),
87-
condition.equal_to('table_schema', ''))
86+
tables = table.TableSchema.where(
87+
condition.equal_to('table_catalog', ''),
88+
condition.equal_to('table_schema', ''),
89+
)
8890
for table_row in tables:
8991
name = table_row.table_name
9092
table_data[name]['parent_table'] = table_row.parent_table_name
@@ -98,9 +100,10 @@ def indexes(cls) -> Dict[str, Dict[str, Any]]:
98100
# Results are ordered by that so the index columns are added in the
99101
# correct order.
100102
index_column_schemas = index_column.IndexColumnSchema.where(
101-
None, condition.equal_to('table_catalog', ''),
103+
condition.equal_to('table_catalog', ''),
102104
condition.equal_to('table_schema', ''),
103-
condition.order_by(('ordinal_position', condition.OrderType.ASC)))
105+
condition.order_by(('ordinal_position', condition.OrderType.ASC)),
106+
)
104107

105108
index_columns = collections.defaultdict(list)
106109
storing_columns = collections.defaultdict(list)
@@ -112,8 +115,9 @@ def indexes(cls) -> Dict[str, Dict[str, Any]]:
112115
storing_columns[key].append(schema.column_name)
113116

114117
index_schemas = index_schema.IndexSchema.where(
115-
None, condition.equal_to('table_catalog', ''),
116-
condition.equal_to('table_schema', ''))
118+
condition.equal_to('table_catalog', ''),
119+
condition.equal_to('table_schema', ''),
120+
)
117121
indexes = collections.defaultdict(dict)
118122
for schema in index_schemas:
119123
key = (schema.table_name, schema.index_name)

‎spanner_orm/admin/migration_executor.py‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,7 @@ def _update_status(self, migration_id: str, new_status: bool) -> None:
165165
'migrated': new_status,
166166
'update_time': datetime.datetime.utcnow(),
167167
})
168-
migration_status.MigrationStatus.save_batch(
169-
None, [new_model], force_write=True)
168+
migration_status.MigrationStatus.save_batch([new_model], force_write=True)
170169
self._migration_status()[migration_id] = new_status
171170

172171
def _validate_migrations(self) -> None:

‎spanner_orm/admin/update.py‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,8 +182,9 @@ def validate(self) -> None:
182182

183183
# Verify no indices exist on the column we're trying to drop
184184
num_indexed_columns = index_column.IndexColumnSchema.count(
185-
None, condition.equal_to('column_name', self._column),
186-
condition.equal_to('table_name', self._table))
185+
condition.equal_to('column_name', self._column),
186+
condition.equal_to('table_name', self._table),
187+
)
187188
if num_indexed_columns > 0:
188189
raise error.SpannerError('Column {} is indexed'.format(self._column))
189190

‎spanner_orm/model.py‎

Lines changed: 44 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,7 @@ def spanner_api(cls) -> api.SpannerApi:
184184
@classmethod
185185
def all(
186186
cls: Type[T],
187+
*,
187188
transaction: Optional[spanner_transaction.Transaction] = None,
188189
) -> List[T]:
189190
"""Returns all objects of this type stored in Spanner.
@@ -206,17 +207,17 @@ def all(
206207
@classmethod
207208
def count(
208209
cls,
209-
transaction: Optional[spanner_transaction.Transaction],
210210
*conditions: condition.Condition,
211+
transaction: Optional[spanner_transaction.Transaction] = None,
211212
) -> int:
212213
"""Returns the number of objects in Spanner that match the given conditions.
213214
214215
Args:
215-
transaction: The existing transaction to use, or None to start a new
216-
transaction
217216
*conditions: Instances of subclasses of Condition that help specify which
218217
rows should be included in the count. The includes condition is not
219218
allowed here
219+
transaction: The existing transaction to use, or None to start a new
220+
transaction
220221
221222
Returns:
222223
The integer result of the COUNT query
@@ -229,6 +230,7 @@ def count(
229230
@classmethod
230231
def count_equal(
231232
cls,
233+
*,
232234
transaction: Optional[spanner_transaction.Transaction] = None,
233235
**constraints: Any,
234236
) -> int:
@@ -253,11 +255,12 @@ def count_equal(
253255
conditions.append(condition.in_list(column, value))
254256
else:
255257
conditions.append(condition.equal_to(column, value))
256-
return cls.count(transaction, *conditions)
258+
return cls.count(*conditions, transaction=transaction)
257259

258260
@classmethod
259261
def find(
260262
cls: Type[T],
263+
*,
261264
transaction: Optional[spanner_transaction.Transaction] = None,
262265
**keys: Any,
263266
) -> Optional[T]:
@@ -273,23 +276,24 @@ def find(
273276
Returns:
274277
The requested object or None if no such object exists
275278
"""
276-
resources = cls.find_multi(transaction, [keys])
279+
resources = cls.find_multi([keys], transaction=transaction)
277280
return resources[0] if resources else None
278281

279282
@classmethod
280283
def find_multi(
281284
cls: Type[T],
282-
transaction: Optional[spanner_transaction.Transaction],
283285
keys: Iterable[Dict[str, Any]],
286+
*,
287+
transaction: Optional[spanner_transaction.Transaction] = None,
284288
) -> List[T]:
285289
"""Retrieves objects from Spanner based on the provided keys.
286290
287291
Args:
288-
transaction: The existing transaction to use, or None to start a new
289-
transaction
290292
keys: An iterable of dictionaries, each dictionary representing the set of
291293
primary key values necessary to uniquely identify an object in this
292294
table.
295+
transaction: The existing transaction to use, or None to start a new
296+
transaction
293297
294298
Returns:
295299
A list containing all requested objects that exist in the table (can be
@@ -307,16 +311,16 @@ def find_multi(
307311
@classmethod
308312
def where(
309313
cls: Type[T],
310-
transaction: Optional[spanner_transaction.Transaction],
311314
*conditions: condition.Condition,
315+
transaction: Optional[spanner_transaction.Transaction] = None,
312316
) -> List[T]:
313317
"""Retrieves objects from Spanner based on the provided conditions.
314318
315319
Args:
316-
transaction: The existing transaction to use, or None to start a new
317-
transaction
318320
*conditions: Instances of subclasses of Condition that help specify which
319321
objects should be retrieved
322+
transaction: The existing transaction to use, or None to start a new
323+
transaction
320324
321325
Returns:
322326
A list containing all requested objects that exist in the table (can be
@@ -330,6 +334,7 @@ def where(
330334
@classmethod
331335
def where_equal(
332336
cls: Type[T],
337+
*,
333338
transaction: Optional[spanner_transaction.Transaction] = None,
334339
**constraints: Any,
335340
) -> List[T]:
@@ -352,7 +357,7 @@ def where_equal(
352357
conditions.append(condition.in_list(column, value))
353358
else:
354359
conditions.append(condition.equal_to(column, value))
355-
return cls.where(transaction, *conditions)
360+
return cls.where(*conditions, transaction=transaction)
356361

357362
@classmethod
358363
def _results_to_models(
@@ -378,6 +383,7 @@ def _execute_read(
378383
@classmethod
379384
def create(
380385
cls,
386+
*,
381387
transaction: Optional[spanner_transaction.Transaction] = None,
382388
**kwargs: Any,
383389
) -> None:
@@ -397,6 +403,7 @@ def create(
397403
@classmethod
398404
def create_or_update(
399405
cls,
406+
*,
400407
transaction: Optional[spanner_transaction.Transaction] = None,
401408
**kwargs: Any,
402409
) -> None:
@@ -418,15 +425,16 @@ def _delete_by_keyset(
418425
@classmethod
419426
def delete_batch(
420427
cls: Type[T],
421-
transaction: Optional[spanner_transaction.Transaction],
422428
models: List[T],
429+
*,
430+
transaction: Optional[spanner_transaction.Transaction] = None,
423431
) -> None:
424432
"""Deletes rows from Spanner based on the provided models' primary keys.
425433
426434
Args:
435+
models: A list of models to be deleted from Spanner.
427436
transaction: The existing transaction to use, or None to start a new
428437
transaction
429-
models: A list of models to be deleted from Spanner.
430438
"""
431439
key_list = []
432440
for model in models:
@@ -439,6 +447,7 @@ def delete_batch(
439447
@classmethod
440448
def delete_by_key(
441449
cls,
450+
*,
442451
transaction: Optional[spanner_transaction.Transaction] = None,
443452
**keys: Any,
444453
) -> None:
@@ -460,20 +469,21 @@ def delete_by_key(
460469
@classmethod
461470
def save_batch(
462471
cls: Type[T],
463-
transaction: Optional[spanner_transaction.Transaction],
464472
models: List[T],
473+
*,
474+
transaction: Optional[spanner_transaction.Transaction] = None,
465475
force_write: bool = False,
466476
) -> None:
467477
"""Writes rows to Spanner based on the provided model data.
468478
469479
Args:
470-
transaction: The existing transaction to use, or None to start a new
471-
transaction
472480
models: A list of models to be written to Spanner. If the _persisted flag
473481
is set, by default we try to issue an UPDATE with values set for all
474482
columns in the table. Otherwise, we try to issue an INSERT for all
475483
columns in the table. If we try to INSERTa row that already exists (or
476484
update one that is missing), an exception will be thrown.
485+
transaction: The existing transaction to use, or None to start a new
486+
transaction
477487
force_write: If true, we use UPSERT instead of UPDATE/INSERT, so no
478488
exceptions are thrown based on the presence or absence of data in
479489
Spanner
@@ -495,6 +505,7 @@ def save_batch(
495505
@classmethod
496506
def update(
497507
cls,
508+
*,
498509
transaction: Optional[spanner_transaction.Transaction] = None,
499510
**kwargs: Any,
500511
) -> None:
@@ -596,7 +607,11 @@ def changes(self) -> Dict[str, Any]:
596607
if values[key] != self.start_values.get(key)
597608
}
598609

599-
def delete(self, transaction: spanner_transaction.Transaction = None) -> None:
610+
def delete(
611+
self,
612+
*,
613+
transaction: Optional[spanner_transaction.Transaction] = None,
614+
) -> None:
600615
"""Deletes this object from the Spanner database.
601616
602617
Args:
@@ -625,7 +640,9 @@ def id(self) -> Dict[str, Any]:
625640

626641
def reload(
627642
self,
628-
transaction: spanner_transaction.Transaction = None) -> Optional['Model']:
643+
*,
644+
transaction: Optional[spanner_transaction.Transaction] = None,
645+
) -> Optional['Model']:
629646
"""Refreshes this object with information from Spanner.
630647
631648
Args:
@@ -637,7 +654,7 @@ def reload(
637654
in Spanner, or None if no information was found (object was deleted or
638655
never was persisted)
639656
"""
640-
updated_object = self._metaclass.find(transaction, **self.id())
657+
updated_object = self._metaclass.find(transaction=transaction, **self.id())
641658
if updated_object is None:
642659
return None
643660
start_values = {}
@@ -652,8 +669,11 @@ def reload(
652669
self._persisted = True
653670
return self
654671

655-
def save(self,
656-
transaction: spanner_transaction.Transaction = None) -> 'Model':
672+
def save(
673+
self,
674+
*,
675+
transaction: Optional[spanner_transaction.Transaction] = None,
676+
) -> 'Model':
657677
"""Persists this object to Spanner.
658678
659679
Note: if the _persisted flag doesn't match whether this object is actually
@@ -671,8 +691,8 @@ def save(self,
671691
changed_values = self.changes()
672692
if changed_values:
673693
changed_values.update(self.id())
674-
self._metaclass.update(transaction, **changed_values)
694+
self._metaclass.update(transaction=transaction, **changed_values)
675695
else:
676-
self._metaclass.create(transaction, **self.values)
696+
self._metaclass.create(transaction=transaction, **self.values)
677697
self._persisted = True
678698
return self

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL