Open NEC-Vishal opened 1 year ago
CLA Assistant Lite bot All contributors have signed the CLA ✍️
@NEC-Vishal this TODO is harder to get right than I initially thought.
There are two problems with the code as it stands in the master branch:
I propose we do the following. (Please feel free to propose a different approach if you don't agree.)
First off, we need an abstract method in the SQLTranslator
class to insert each and every batch regardless of whether any previous inserts failed.
class SQLTranslator:
def _insert_row_batches(stmt: str, batches: [[Tuple]]):
raise NotImplementedError
def _insert_entity_rows(...
...
try:
batches = to_insert_batches(rows)
self._insert_row_batches(stmt, batches)
except Exception as e:
...
In the case of one or multiple batches failing, _insert_row_batches
will throw an exception. If there are multiple failures, they all get wrapped up in a single exception. This is obviously far from optimal, but at least the existing error handling code in _insert_entity_rows
will keep on working as expected, in particular the last-ditch attempt to save entity data in the original format.
Then this should be the Crate-specific implementation.
class CrateTranslator:
def _insert_row_batches(stmt: str, batches: [[Tuple]]):
has_error = False
for batch in batches:
res = self.cursor.executemany(stmt, batch)
# recent Crate versions don't bomb out anymore when
# something goes wrong with a multi-row insert, instead
# the driver returns -2 for each row that has an issue
if isinstance(res, list):
for i in range(len(res)):
if res[i]['rowcount'] < 0:
has_error = True
if has_error:
raise Exception('An insert failed')
Whereas for Timescale, we could have something like this
class PostgresTranslator:
def _insert_row_batches(stmt: str, batches: [[Tuple]]):
error = None
for batch in batches:
try:
self.cursor.executemany(stmt, batch)
except Exception as e:
error = e
if error:
raise error
@NEC-Vishal please keep in mind I haven't tested any of this and the pseudo-code I sketched out earlier may need some tweaking. But the biggest chunk of work in this PR will be the testing. This is a critical section of the code base, so you'll need to implement test cases to cover all possible scenarios---no failure, partial failure, total failure. Both for Crate and Timescale...
Proposed changes
697 improve error handling in sql_translator.py
Types of changes
What types of changes does your code introduce to the project? Put an
x
in the boxes that applyChecklist
Put an
x
in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...