gdal icon indicating copy to clipboard operation
gdal copied to clipboard

Python bindings: don't throw exception in StartTransaction and friends if transaction is not supported

Open theroggy opened this issue 9 months ago • 4 comments

Feature description

StartTransaction and friends (CommitTransaction and RollbackTransaction) throw an exception in python if the underlying datasource doesn't support transactions.

This gives these kind of constructs that need to be used everywhere in code that supports both datasources with and without transaction support:

if datasource.TestCapability(gdal.ogr.ODsCTransactions):
    datasource.StartTransaction()

I'm not sure if there is an actual added value in the exceptions being thrown...

The functions could return True or False depending of the transaction start being successful or not or, if the default behaviour of throwing an exception is wanted, an extra parameter that no exception should be thrown is also an option?

Additional context

No response

theroggy avatar Mar 07 '25 16:03 theroggy

DataSource.StartTransaction is respecting the value of ogr.UseExceptions(), which seems like expected behavior to me. If the alternative of checking return values is desired, why not disable exceptions?

dbaston avatar Mar 07 '25 17:03 dbaston

Typically you want to have exceptions being thrown everywhere where explicitly relevant... so having to disable them specifically for these calls is not really an improvement compared to having to use TestCapability everywhere to avoid running the functions...

theroggy avatar Mar 07 '25 17:03 theroggy

so having to disable them specifically for these calls

then

try:
    datasource.StartTransaction()
except RuntimeError:
    pass

And if that is too verbose, put it in a function start_transaction_if_supported(ds) ? But that seems too niche to live inside GDAL.

dbaston avatar Mar 21 '25 15:03 dbaston

I'd like not to have an exception when using the transaction functions for datasources that don't support transactions, but having an exception when there is actually an error happening when trying to start/commit/rollback a transaction.

I'm using the helper functions below now to get ~this behaviour, and I thought something like that would be a better default behaviour... but obviously that's subjective.

def StartTransaction(datasource: gdal.Dataset) -> bool:
    """Starts a transaction on an open datasource.

    Args:
        datasource (gdal.Dataset): the datasource to start the transaction on.

    Raises:
        ValueError: if datasource is None.

    Returns:
        bool: True if the transaction was started successfully.
    """
    if datasource is None:
        raise ValueError("datasource is None")

    if datasource.TestCapability(ogr.ODsCTransactions):
        datasource.StartTransaction()

    return True


def CommitTransaction(datasource: gdal.Dataset | None) -> bool:
    """Commits a transaction on an open datasource.

    Args:
        datasource (gdal.Dataset): the datasource to commit the transaction on. If None,
            no commit is executed.

    Returns:
        bool: True if the transaction was committed successfully.
    """
    if datasource is None:
        return False

    if datasource.TestCapability(ogr.ODsCTransactions):
        datasource.CommitTransaction()

    return True


def RollbackTransaction(datasource: gdal.Dataset | None) -> bool:
    """Rolls back a transaction on an open datasource.

    Args:
        datasource (gdal.Dataset): the datasource to roll back the transaction on. If
            None, no rollback is executed.

    Returns:
        bool: True if the transaction was rolled back successfully.
    """
    if datasource is None:
        return False

    if datasource.TestCapability(ogr.ODsCTransactions):
        datasource.RollbackTransaction()

    return True

theroggy avatar Mar 21 '25 17:03 theroggy