experiment in quit callback and decoupling transactions from postgres #2

Closed
wants to merge 8 commits into
from

Conversation

Projects
None yet
1 participant
@twobraids

this is only an experiment to get feedback. Yeah, I should have decoupled these two issues, but I didn't.

take a look at the changes to how I use the transaction class in your PostgresCrashStorage class. Rather than the transaction instantiating its own database connection context, the connection context is instantiated separately. I prefer this decoupling because it gives the designer more freedom in placing the database connections into any namespace rather than forcing it to be within the transaction namespace.

take a look around and let's discuss it.

+ def __init__(self, config, quit_check_callback):
+ super(FileSystemCrashStorage, self).__init__(
+ config,
+ quit_check_callback=lambda: False

This comment has been minimized.

Show comment Hide comment
@twobraids

twobraids Mar 27, 2012

that looks like an error to me. This class should pass the quit_check_callback to the parent class, not override it.

@twobraids

twobraids Mar 27, 2012

that looks like an error to me. This class should pass the quit_check_callback to the parent class, not override it.

+ def __init__(self, config, quit_check_callback):
+ super(FileSystemThrottledCrashStorage, self).__init__(
+ config,
+ quit_check_callback=lambda: False

This comment has been minimized.

Show comment Hide comment
@twobraids

twobraids Mar 27, 2012

that looks like an error to me. This class should pass the quit_check_callback to the parent class, not override it.

@twobraids

twobraids Mar 27, 2012

that looks like an error to me. This class should pass the quit_check_callback to the parent class, not override it.

#--------------------------------------------------------------------------
- def __init__(self, config):
+ def __init__(self, config, db_conn_context_source,
+ abandonment_callback=None):

This comment has been minimized.

Show comment Hide comment
@twobraids

twobraids Mar 27, 2012

This is inconsistent, sometimes it is called abandonment_callback and other times it is quit_check_callback

@twobraids

twobraids Mar 27, 2012

This is inconsistent, sometimes it is called abandonment_callback and other times it is quit_check_callback

socorro/database/transaction_executor.py
with self.config.db_connection_context() as connection:
try:
function(connection, *args, **kwargs)
connection.commit()
except:
- if connection.get_transaction_status() == \
- psycopg2.extensions.TRANSACTION_STATUS_INTRANS:
+ if db_conn_context_source.in_transaction(connection):

This comment has been minimized.

Show comment Hide comment
@twobraids

twobraids Mar 27, 2012

got to remember to remove the import of psycopg2.extensions, too.

@twobraids

twobraids Mar 27, 2012

got to remember to remove the import of psycopg2.extensions, too.

@twobraids twobraids closed this Apr 5, 2012

@twobraids

This comment has been minimized.

Show comment Hide comment
@twobraids

twobraids Apr 5, 2012

withdrawn - will be broken into little bits for separate reviews...

withdrawn - will be broken into little bits for separate reviews...

peterbe pushed a commit that referenced this pull request Apr 13, 2012

Merge pull request #2 from peterbe/hbase2012
accepting the patch - thanks

peterbe pushed a commit that referenced this pull request Aug 31, 2016

Merge pull request #2 from adngdb/bug1035144
Removed inheritence from SignatureGenerationRule.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment