Injecting User info into _history table to track who performed the change

[JPLaverdure]

Hello everyone,

We already have the ability to timestamp the creation of the history row, but it would also be interesting to be able to track the user responsible for the content update. 

I would like to get suggestions on the best way to achieve this.

I realize this is somewhat outside the scope of sqlalchemy as the notion of a "logged in user" is more closely related to the context of the app/webapp using SQLAlchemy as its ORM but maybe other people would benefit from having a way to inject arbitrary data in the history table.

Ideally, I would like the insert in the _history table to be atomic, so I feel like hooking an update statement to an event might not be the way to go.

I'm tempted to modify the signature of before_flush but I'm not sure where it gets called.

Any help is welcome !

Thanks

JP

[Elmer de Looff]

Hi JP,

Depending on how you've implemented your history tracking, that routine is quite far removed from your web framework and getting a neat, clean way of dealing with that might not be within reach.

However, most web frameworks have some concept of a threadlocal request (or function to retrieve it), which you could invoke and if such a request exists, you could use that to load whatever user identity you have available on there (again, the details differ, but this tends to be a shared feature). From there you can store the user either as a foreign key, or a unique identifier like email. Which one you pick would depend on how you want the history to be affected when you delete a user record for example.

--
SQLAlchemy -
The Python SQL Toolkit and Object Relational Mapper
 
http://www.sqlalchemy.org/
 
To post example code, please provide an MCVE: Minimal, Complete, and Verifiable Example. See http://stackoverflow.com/help/mcve for a full description.
---
You received this message because you are subscribed to the Google Groups "sqlalchemy" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sqlalchemy+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/82a24998-14e1-4ff4-a725-dd25c20a8bf2n%40googlegroups.com.

--

Elmer

[JPLaverdure]

Hi Elmer,

Thanks for your reply !

My issue is not with obtaining the info I want to inject (the logged in users's email), I already have that all ready to go :)

My whole database is versioned using the history_meta.py example from SQLAlchemy

https://docs.sqlalchemy.org/en/13/_modules/examples/versioned_history/history_meta.html

I was hoping for a simple way to inject the user info into the _history row creation steps.

The SQLAlchemy example makes use of this event listener:

def versioned_session(session):

    @event.listens_for(session, "before_flush") 

    def before_flush(session, flush_context, instances): 

        for obj in versioned_objects(session.dirty): 

            create_version(obj, session) 

       for obj in versioned_objects(session.deleted): 

            create_version(obj, session, deleted=True)

So I'm tempted to follow the same strategy and just override this listener to supplement it with the user info but I'm wondering how to pass in non SQLAlchemy info into its execution context...

So basically, I have the info I want to inject, I'm just not sure how to pass it to SQLAlchemy

Thanks,

JP

[Simon King]

I use pyramid as a web framework, and when I create the DB session for
each request, I add a reference to the current request object to the
DB session. The session object has an "info" attribute which is
intended for application-specific things like this:

https://docs.sqlalchemy.org/en/13/orm/session_api.html#sqlalchemy.orm.session.Session.info

Then, in the before_flush event handler, I retrieve the request object
from session.info, and then I can add whatever request-specific info I
want to the DB.

Simon

> To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/58bb6713-18f4-4d69-8d7b-a27772711bd5n%40googlegroups.com.

[Jean-Philippe Laverdure]

Ohhh, that sounds perfect !

And since I already make use of class based views, that should be dead simple to put in place.

Thanks for the support, guys

Cheers,

JP

You received this message because you are subscribed to a topic in the Google Groups "sqlalchemy" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/sqlalchemy/viS8s1sXnNQ/unsubscribe.
To unsubscribe from this group and all its topics, send an email to sqlalchemy+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/CAFHwexcy12htOqEeJGeGf%3D-oxm7ynRbnmFV8AvgW0QusZoJ8hA%40mail.gmail.com.

--

Jean-Philippe Laverdure

C: 514-923-7401

[Jonathan Vanasco]

Going beyond what Simon did..

I typically make make a table like `user_transaction`, which has all of the relevant information for the transaction:

* User ID

* Timestamp
* Remote IP

Using the sqlalchemy hooks, I'll then do something like:

* update the object table with the user_transaction id
or
* use an association table that tracks a user_transaction_id to an object id and version
 
FYI, Simon -- as of a few weeks ago, that pattern is now part of the pyramid sqlalchemy starter template!

[JPLaverdure]

Hi,

Thanks for your support guys.

I've implemented logging the user's email in the _history tables by adding this Column definition inside my history_meta.py file:

cols.append(

  Column(

    "user",

    String,

    info=version_meta,

  )

)

but i'm running into a good load of SAWarnings stating that there is an implicit combining of the "user" column taking place

(I have multi-table inheritance setup for some entities, those are the ones throwing the warning)

I don't get why the column "changed" (which holds the timestamp of the change) and is defined in exactly the same way does not generate these warnings ?

What configuration setting am I missing here ?

I found this 

https://docs.sqlalchemy.org/en/13/faq/ormconfiguration.html#i-m-getting-a-warning-or-error-about-implicitly-combining-column-x-under-attribute-y

But it doesn't seem to fit 100% with what I'm seeing inside history_meta.py

Thanks !!

[Simon King]

Can you show us the mapping definitions that are triggering these warnings?

> To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/262ddd37-0dc9-410e-94a0-25b8a82730e0n%40googlegroups.com.

[JPLaverdure]

It seems I lost my previous email.. Here it is again:

Sure !

Here are 2 classes for which the generated _history sister tables (generated by history_meta.py) throw the warnings:

The Parent class:

class Process(Versioned, Base, UtilityMixin):

__tablename__ = 'process'

__table_args__ = {}

id = Column(Integer, primary_key=True)

discriminator = Column('type', String(64))

timestamp = Column(DateTime, nullable=False)

start_date = Column(Date, nullable=False)

am_pm = Column(String(8))

probable_end_date = Column(Date)

end_date = Column(Date)

user = Column(String(128), nullable=False)

comments = Column(Text)

__mapper_args__ = {'polymorphic_on': discriminator, 'polymorphic_identity': 'process',

'order_by': [start_date.desc(), id.desc()]}

protocol_id = Column(Integer, ForeignKey('protocol.id', onupdate='cascade', ondelete='restrict'))

process_type_id = Column(Integer, ForeignKey('process_type.id', onupdate='cascade', ondelete='restrict'))

protocol = relationship(Protocol, backref='processes', uselist=False)

process_type = relationship(ProcessType, backref='processes', uselist=False)

The Child class:

class CompoundAdministration(Process):

__tablename__ = 'compound_administration'

__table_args__ = {}

__mapper_args__ = {'polymorphic_identity': 'compound_admin'}

id = Column(Integer, ForeignKey('process.id', onupdate='cascade', ondelete='cascade'), primary_key=True)

dose = Column(String(64))

substance = Column(String(128))

frequency = Column(String(64))

duration = Column(String(64))

route_id = Column(Integer, ForeignKey('administration_route.id', onupdate='cascade', ondelete='restrict'))

route = relationship(AdministrationRoute, uselist=False)

As reminder, versioning was implemented using this recipe/example from SQLA:

https://docs.sqlalchemy.org/en/13/_modules/examples/versioned_history/history_meta.html

And here is the associated warning:

SAWarning: 

Implicitly combining column process_history.user with column compound_administration_history.user under attribute 'user'.  

Please configure one or more attributes for these same-named columns explicitly.

Thanks for your help resolving this,

JP

[Simon King]

I think the warning message is slightly misleading, probably because
of the inheritance, but the fundamental problem is likely that your
"process" table already has a "user" column, and you're trying to add
a second "user" column in the process_history table, which can't work.

If you use a different column name to store the user in the history
table, does the warning go away?

Simon

> To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/69246876-4fa1-4824-8ac0-c1f41bf779f7n%40googlegroups.com.

[JPLaverdure]

Hi Simon,

Thanks for pointing out the collision, it kinda flew under the radar !

I renamed the column from "user" to "accountable" and but still got 

SAWarning: 

Implicitly combining column process_history.accountable with column compound_administration_history.accountable under attribute 'accountable'.  

Please configure one or more attributes for these same-named columns explicitly.

As mentioned, these tables both also have a "changed" attribute, but did not throw the warnings...

Looking a bit further, I spotted this piece of code in history_meta:

properties["changed"] = (table.c.changed,) + tuple(

    super_history_mapper.attrs.changed.columns

)

So I added:

properties["accountable"] = (table.c.accountable,) + tuple(

    super_history_mapper.attrs.accountable.columns

)

And the warnings have disappeared.

Could you explain what these instructions actually do ?

Thanks

[Simon King]

I'm not 100% confident here (the history_meta code does some pretty
complicated stuff to deal with inheritance), but I'll give it a go:

1. You've adapted history_meta to add an extra column to history
tables, called "accountable".
2. You've got an inheritance hierarchy (CompoundAdministration
inherits from Process)
3. Both CompoundAdministration and Process use the Versioned mixin
4. This results in 2 new mapped classes, ProcessHistory and
CompoundAdministrationHistory.
5. CompoundAdministrationHistory inherits from ProcessHistory

Step 5 is the problem: CompoundAdministrationHistory inherits from
ProcessHistory, but both tables have an "accountable" column. Normally
that's a problem - when you set the
CompoundAdministrationHistory.accountable attribute, should SQLAlchemy
update the column in the compound_administration_history table, or
process_history, or both? SQLAlchemy defaults to updating both, but
warns you about the ambiguity.

In this case, you really do want that property to target both columns,
so your fix is correct:

properties["accountable"] = (table.c.accountable,) + tuple(
super_history_mapper.attrs.accountable.columns
)

This says that the "accountable" property should target the column on
the local (inheriting) table as well as whatever columns the parent
class was targeting.

You should test that when you modify a CompoundAdministration object,
you get a new row in both the compound_administration_history and the
process_history tables, and that the "accountable" column is set
correctly.

I hope that makes sense,

Simon

> To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/8f4f0d74-98f6-497e-ac4c-35dae86fe1f6n%40googlegroups.com.

[JPLaverdure]

Yup, makes perfect sense.

So what the code/fix is achieving, in effect, is what is described here:

https://docs.sqlalchemy.org/en/13/faq/ormconfiguration.html#i-m-getting-a-warning-or-error-about-implicitly-combining-column-x-under-attribute-y

Just using a different approach because of the inheritance. 

Cheers,

JP