2

The Situation: Simple Class with Basic Attributes

In an application I'm working on, instances of particular class are persisted at the end of their lifecycle, and while they are not subsequently modified, their attributes may need to be read. For example, the end_time of the instance or its ordinal position relative to other instances of the same class (first instance initialized gets value 1, the next has value 2, etc.).

class Foo(object):
    def __init__(self, position):
        self.start_time = time.time()
        self.end_time = None
        self.position = position
        # ...
    def finishFoo(self):
        self.end_time = time.time()
        self.duration = self.end_time - self.start_time
    # ...

The Goal: Persist an Instance using SQLAlchemy

Following what I believe to be a best practice - using a scoped SQLAlchemy Session, as suggested here, by way of contextlib.contextmanager - I save the instance in a newly-created Session which immediately commits. The very next line references the newly persistent instance by mentioning it in a log record, which throws a DetachedInstanceError because the attribute its referencing expired when the Session committed.

class Database(object):
    # ...
    def scopedSession(self):
        session = self.sessionmaker()
        try:
            yield session
            session.commit()
        except:
            session.rollback()
            logger.warn("blah blah blah...")
        finally:
            session.close()
    # ...
    def saveMyFoo(self, foo):
        with self.scopedSession() as sql_session:
            sql_session.add(foo)
        logger.info("Foo number {0} finished at {1} has been saved."
                    "".format(foo.position, foo.end_time))
        ## Here the DetachedInstanceError is raised

Two Known Possible Solutions: No Expiring or No Scope

I know I can set the expire_on_commit flag to False to circumvent this issue, but I'm concerned this is a questionable practice -- automatic expiration exists for a reason, and I'm hesitant to arbitrarily lump all ORM-tied classes into a non-expiry state without sufficient reason and understanding behind it. Alternatively, I can forget about scoping the Session and just leave the transaction pending until I explicitly commit at a (much) later time.

So my question boils down to this:

  1. Is a scoped/context-managed Session being used appropriately in the case I described?
  2. Is there an alternative way to reference expired attributes that is a better/more preferred approach? (e.g. using a property to wrap the steps of catching expiration/detached exceptions or to create & update a non-ORM-linked attribute that "mirrors" the ORM-linked expired attribute)
  3. Am I misunderstanding or misusing the SQLAlchemy Session and ORM? It seems contradictory to me to use a contextmanager approach when that precludes the ability to subsequently reference any of the persisted attributes, even for a task as simple and broadly applicable as logging.

The Actual Exception Traceback

The example above is simplified to focus on the question at hand, but should it be useful, here's the actual exact traceback produced. The issue arises when str.format() is run in the logger.debug() call, which tries to execute the Set instance's __repr__() method.

Unhandled Error
Traceback (most recent call last):
  File "/opt/zenith/env/local/lib/python2.7/site-packages/twisted/python/log.py", line 73, in callWithContext
    return context.call({ILogContext: newCtx}, func, *args, **kw)
  File "/opt/zenith/env/local/lib/python2.7/site-packages/twisted/python/context.py", line 118, in callWithContext
    return self.currentContext().callWithContext(ctx, func, *args, **kw)
  File "/opt/zenith/env/local/lib/python2.7/site-packages/twisted/python/context.py", line 81, in callWithContext
    return func(*args,**kw)
  File "/opt/zenith/env/local/lib/python2.7/site-packages/twisted/internet/posixbase.py", line 614, in _doReadOrWrite
    why = selectable.doRead()
--- <exception caught here> ---
  File "/opt/zenith/env/local/lib/python2.7/site-packages/twisted/internet/udp.py", line 248, in doRead
    self.protocol.datagramReceived(data, addr)
  File "/opt/zenith/operations/network.py", line 311, in datagramReceived
    self.reactFunction(datagram, (host, port))
  File "/opt/zenith/operations/schema_sqlite.py", line 309, in writeDatapoint
    logger.debug("Data written: {0}".format(dataz))
  File "/opt/zenith/operations/model.py", line 1770, in __repr__
    repr_info = "Set: {0}, User: {1}, Reps: {2}".format(self.setNumber, self.user, self.repCount)
  File "/opt/zenith/env/local/lib/python2.7/site-packages/sqlalchemy/orm/attributes.py", line 239, in __get__
    return self.impl.get(instance_state(instance), dict_)
  File "/opt/zenith/env/local/lib/python2.7/site-packages/sqlalchemy/orm/attributes.py", line 589, in get
    value = callable_(state, passive)
  File "/opt/zenith/env/local/lib/python2.7/site-packages/sqlalchemy/orm/state.py", line 424, in __call__
    self.manager.deferred_scalar_loader(self, toload)
  File "/opt/zenith/env/local/lib/python2.7/site-packages/sqlalchemy/orm/loading.py", line 563, in load_scalar_attributes
    (state_str(state)))
sqlalchemy.orm.exc.DetachedInstanceError: Instance <Set at 0x1c96b90> is not bound to a Session; attribute refresh operation cannot proceed
radzak
  • 2,986
  • 1
  • 18
  • 27
TCAllen07
  • 1,294
  • 16
  • 27

1 Answers1

4

1.

Most likely, yes. It's used correctly insofar as correctly saving data to the database. However, because your transaction only spans the update, you may run into race conditions when updating the same row. Depending on the application, this can be okay.

2.

Not expiring attributes is the right way to do it. The reason the expiration is there by default is because it ensures that even naive code works correctly. If you are careful, it shouldn't be a problem.

3.

It's important to separate the concept of the transaction from the concept of the session. The contextmanager does two things: it maintains the session as well as the transaction. The lifecycle of each ORM instance is limited to the span of each transaction. This is so you can assume the state of the object is the same as the state of the corresponding row in the database. This is why the framework expires attributes when you commit, because it can no longer guarantee the state of the values after the transaction commits. Hence, you can only access the instance's attributes while a transaction is active.

After you commit, any subsequent attribute you access will result in a new transaction being started so that the ORM can once again guarantee the state of the values in the database.

But why do you get an error? This is because your session is gone, so the ORM has no way of starting a transaction. If you do a session.commit() in the middle of your context manager block, you'll notice a new transaction being started if you access one of the attributes.

Well, what if I want to just access the previously-fetched values? Then, you can ask the framework not to expire those attributes.

univerio
  • 19,548
  • 3
  • 66
  • 68
  • Your explanation is about the best I could imagined -- thanks for being so thorough and yet clear. I wanted to ask a couple quick follow-ups: On (1), given the app only ever persists one instance one time, never updating the same row, this would avoid any such race conditions, right? And on (3) were I to commit the session but not *close* it, that would finish the transaction but leave the session, and thus not cause the detachment -- does that sound correct? – TCAllen07 Feb 10 '16 at 22:05
  • I think I answered my second follow-up. I tossed the `finally: session.close()` and the `DetachedInstanceError` is not raised. Probably should have tried that before commenting. But I am still curious about the first follow-up. – TCAllen07 Feb 10 '16 at 22:09
  • 1
    @TCAllen07 If you ever read from the database then write back to it, it is possible to have race conditions. If you *only* ever write, it should be fine. Regarding the second question, you need to close the session *at some point*. This frees up connection resources. You can delay this until you no longer need the instances in the session (e.g. in a REST app, the session is usually started at the beginning of each request and closed at the end), but you shouldn't just leave it there. (The garbage collector will *probably* dispose of it at some point, but don't rely on that.) – univerio Feb 10 '16 at 22:39