Re: psycopg2: proper positioning of .commit() within try: except: blocks

2024-09-08 Thread Karsten Hilbert via Python-list
Am Sun, Sep 08, 2024 at 12:48:50PM +1200 schrieb Greg Ewing via Python-list:

> On 8/09/24 9:20 am, Karsten Hilbert wrote:
> > try:
> > do something
> > except:
> > log something
> > finally:
> > .commit()
> >
> >cadence is fairly Pythonic and elegant in that it ensures the
> >the .commit() will always be reached regardless of exceptions
> >being thrown or not and them being handled or not.
>
> That seems wrong to me. I would have thought the commit should only
> be attempted if everything went right.
>
> What if there's a problem in your code that causes a non-SQL-related
> exception when some but not all of the SQL statements in the
> transaction bave been issued? The database doesn't know something
> has gone wrong, so it will happily commit a partially-completed
> transaction and possibly corrupt your data.

A-ha !

try:
run_some_SQL_that_succeeds()
print(no_such_name) # 
tongue-in-cheek
1 / 0   # for 
good measure
except SOME_DB_ERROR:
print('some DB error, can be ignored for now')
finally:
commit()

which is wrong, given that the failing *Python* statements
may very well belong into the *business level* "transaction"
which a/the database transaction is part of.

See, that's why I was asking in the first place :-)

I was overlooking implications.

Karsten
--
GPG  40BE 5B0E C98E 1713 AFA6  5BC0 3BEA AC80 7D4F C89B
-- 
https://mail.python.org/mailman/listinfo/python-list


Re: psycopg2: proper positioning of .commit() within try: except: blocks

2024-09-08 Thread Karsten Hilbert via Python-list
Am Sun, Sep 08, 2024 at 12:48:50PM +1200 schrieb Greg Ewing via Python-list:

> On 8/09/24 9:20 am, Karsten Hilbert wrote:
> > try:
> > do something
> > except:
> > log something
> > finally:
> > .commit()
> >
> >cadence is fairly Pythonic and elegant in that it ensures the
> >the .commit() will always be reached regardless of exceptions
> >being thrown or not and them being handled or not.
>
> That seems wrong to me. I would have thought the commit should only
> be attempted if everything went right.

It is only attempted when "everything" went right. The fault
in my thinking was what the "everything" might encompass.
When some SQL fails it won't matter whether I say
conn.commit() or conn.rollback() or, in fact, nothing at all
-- the (DB !) transaction will be rolled back in any case.

However, that reasoning missed this:

> What if there's a problem in your code that causes a non-SQL-related
> exception when some but not all of the SQL statements in the
> transaction bave been [-- even successfully --] issued?

Still, in this code pattern:

>   try:
> do something
> .commit()
>   except:
> log something
it doesn't technically matter whether I say .commit or .rollback here:
> .rollback()

... but ...

> Doing an explicit rollback ensures that the transaction is always
> rolled back if it is interrupted for any reason.

explicit is better than implicit ;-)

Karsten
--
GPG  40BE 5B0E C98E 1713 AFA6  5BC0 3BEA AC80 7D4F C89B
-- 
https://mail.python.org/mailman/listinfo/python-list


Re: psycopg2: proper positioning of .commit() within try: except: blocks

2024-09-08 Thread Rob Cliffe via Python-list



On 07/09/2024 22:20, Karsten Hilbert via Python-list wrote:

Am Sat, Sep 07, 2024 at 02:09:28PM -0700 schrieb Adrian Klaver:


Right, and this was suggested elsewhere ;)

And, yeah, the actual code is much more involved :-D


I see that.

The question is does the full code you show fail?

The code sample you show in your original post is doing something very 
different then
what you show in your latest post. At this point I do not understand the exact 
problem
we are dealing with.

We are not dealing with an unsolved problem. I had been
asking for advice  where to best place that .commit() call in
case I am overlooking benefits and drawbacks of choices.

The

try:
do something
except:
log something
finally:
.commit()

cadence is fairly Pythonic and elegant in that it ensures the
the .commit() will always be reached regardless of exceptions
being thrown or not and them being handled or not.

It is also insufficient because the .commit() itself may
elicit exceptions (from the database).

So there's choices:

Ugly:

try:
do something
except:
log something
finally:
try:
.commit()
except:
log some more

Fair but not feeling quite safe:

try:
do something
.commit()
except:
log something

Boring and repetitive and safe(r):

try:
do something
except:
log something
try:
.commit()
except:
log something

I eventually opted for the last version, except for factoring
out the second try: except: block.

Best,
Karsten
--
GPG  40BE 5B0E C98E 1713 AFA6  5BC0 3BEA AC80 7D4F C89B
Unless I'm missing something, the 1st & 3rd versions always do the 
commit() even after the first bit fails, which seems wrong.
I suggest the 1st version but replacing "finally" by "else".  Then the 
try-commit-except will not be executed if the "something" fails.
Perhaps the extra indentation of the second try block is a bit ugly, but 
it is more important that it does the right thing.
If it is convenient (it may not be) to put the whole thing in a 
function, you may feel that the follwing is less ugly:


try:
do something
except:
log something
return
try:
.commit()
except:
log some more
return

Best wishes
Rob Cliffe

--
https://mail.python.org/mailman/listinfo/python-list


Re: psycopg2: proper positioning of .commit() within try: except: blocks

2024-09-08 Thread Karsten Hilbert via Python-list
Am Sun, Sep 08, 2024 at 02:58:03PM +0100 schrieb Rob Cliffe via Python-list:

> >Ugly:
> >
> > try:
> > do something
> > except:
> > log something
> > finally:
> > try:
> > .commit()
> > except:
> > log some more
> >
> >Fair but not feeling quite safe:
> >
> > try:
> > do something
> > .commit()
> > except:
> > log something
> >
> >Boring and repetitive and safe(r):
> >
> > try:
> > do something
> > except:
> > log something
> > try:
> > .commit()
> > except:
> > log something
> >
> >I eventually opted for the last version, except for factoring
> >out the second try: except: block.

> Unless I'm missing something, the 1st & 3rd versions always do the commit() 
> even after
> the first bit fails, which seems wrong.

Well, it does run a Python function called "commit". That
function will call "COMMIT" on the database. The end result
will be correct (at the SQL level) because the COMMIT will
not effect a durable commit of data when the SQL in "do
something" had failed.

We have, however, elicited that there may well be other
things belonging into the running business level transaction
which may fail and which should lead to data not being
committed despite being technically correct at the SQL level.

> I suggest the 1st version but replacing "finally" by "else".  Then the 
> try-commit-except
> will not be executed if the "something" fails.

The whole point was to consolidate the commit into one place.
It is unfortunately named, though. It should be called
"close_transaction".

> Perhaps the extra indentation of the second try block is a bit ugly, but it 
> is more
> important that it does the right thing.
> If it is convenient (it may not be) to put the whole thing in a function, you 
> may feel
> that the follwing is less ugly:

The whole thing does reside inside a function but the exit-early pattern

>   try:
>   do something
>   except:
>   log something
>   return
>   try:
>   .commit()
>   except:
>   log some more
>   return

won't help as there's more stuff to be done inside that function.

Thanks,
Karsten


For what it's worth here's the current state of code:

#
def __safely_close_cursor_and_rollback_close_conn(close_cursor=None, 
rollback_tx=None, close_conn=None):
if close_cursor:
try:
close_cursor()
except PG_ERROR_EXCEPTION as pg_exc:
_log.exception('cannot close cursor')
gmConnectionPool.log_pg_exception_details(pg_exc)
if rollback_tx:
try:
# need to rollback so ABORT state isn't retained in 
pooled connections
rollback_tx()
except PG_ERROR_EXCEPTION as pg_exc:
_log.exception('cannot rollback transaction')
gmConnectionPool.log_pg_exception_details(pg_exc)
if close_conn:
try:
close_conn()
except PG_ERROR_EXCEPTION as pg_exc:
_log.exception('cannot close connection')
gmConnectionPool.log_pg_exception_details(pg_exc)

#
def __log_notices(notices_accessor=None):
for notice in notices_accessor.notices:
_log.debug(notice.replace('\n', '/').replace('\n', '/'))
del notices_accessor.notices[:]

#
def __perhaps_reraise_as_permissions_error(pg_exc, curs):
if pg_exc.pgcode != PG_error_codes.INSUFFICIENT_PRIVILEGE:
return

# privilege problem -- normalize as GNUmed exception
details = 'Query: [%s]' % curs.query.decode(errors = 
'replace').strip().strip('\n').strip().strip('\n')
if curs.statusmessage != '':
details = 'Status: %s\n%s' % (

curs.statusmessage.strip().strip('\n').strip().strip('\n'),
details
)
if pg_exc.pgerror is None:
msg = '[%s]' % pg_exc.pgcode
else:
msg = '[%s]: %s' % (pg_exc.pgcode, pg_exc.pgerror)
raise gmExceptions.AccessDenied (
msg,
source = 'PostgreSQL',
code = pg_exc.pgcode,
details = details
)

#
def run_rw_queries (
link_obj:_TLnkObj=None,
queries:_TQueries=None,
end_tx:bool=False,
return_data:bool=None,
get_col_idx:bool=False,
verbo

Re: psycopg2: proper positioning of .commit() within try: except: blocks

2024-09-08 Thread Greg Ewing via Python-list

On 8/09/24 11:03 pm, Jon Ribbens wrote:

On 2024-09-08, Greg Ewing  wrote:

try:
  do something
  .commit()
except:
  log something
  .rollback()


What if there's an exception in your exception handler? I'd put the
rollback in the 'finally' handler, so it's always called.


Good point. Putting the rollback first would be safer/

--
Greg
--
https://mail.python.org/mailman/listinfo/python-list


Re: psycopg2: proper positioning of .commit() within try: except: blocks

2024-09-08 Thread Greg Ewing via Python-list

On 9/09/24 2:13 am, Karsten Hilbert wrote:

For what it's worth here's the current state of code:


That code doesn't inspire much confidence in me. It's far too
convoluted with too much micro-management of exceptions.

I would much prefer to have just *one* place where exceptions are
caught and logged.

--
Greg
--
https://mail.python.org/mailman/listinfo/python-list