Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accessing the history of a synonym attribute (either raise or resolve for attribute) #3777

Closed
sqlalchemy-bot opened this issue Aug 20, 2016 · 5 comments
Labels
bug Something isn't working low priority orm
Milestone

Comments

@sqlalchemy-bot
Copy link
Collaborator

Migrated issue, originally created by Konsta Vesterinen (@kvesteri)

Consider the following code:

import sqlalchemy as sa
from sqlalchemy.ext.declarative import declarative_base, synonym_for

Base = declarative_base()


class User(Base):
    __tablename__ = 'user'
    _name = sa.Column(sa.String, primary_key=True)
    
    @synonym_for('_name')
    @property
    def name(self):
        return self._name

u = User()
sa.inspect(u).attrs['name'].history

Currently this raises the following error:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/sqlalchemy/orm/state.py", line 684, in history
    PASSIVE_NO_INITIALIZE)
  File "/sqlalchemy/orm/state.py", line 309, in get_history
    return self.manager[key].impl.get_history(self, self.dict, passive)
AttributeError: '_ProxyImpl' object has no attribute 'get_history'

I think it would make more sense if this either

  1. Would not raise an AttributeError but rather show the history of the property that the synonym property is pointing at (in this case the history of _name)
  2. Raise an error with more descriptive error message

This is just a minor issue for me and its easy to find a workaround for this.

@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

if someone is iterating through attributes and calling get_history() on all of them, they're going to get the same history multiple times if they hit the synonym(s) then the real attribute, raising would encourage them to iterate only through primary attributes.

OTOH, we do know the answer to the question "some_synonym.get_history()". would help in pdb situations and such.

it's a strange use case because the synonym is a proxy for the real attribute but it's not included in the persistence operation.

I was leaning towards raise and now im leaning towards implementing. but that's just within a 2 minute span so I'll let this sit out there :)

@sqlalchemy-bot
Copy link
Collaborator Author

Changes by Michael Bayer (@zzzeek):

  • changed title from "Accessing the history of a synonym attribute" to "Accessing the history of a synonym attribute (ei"

@sqlalchemy-bot
Copy link
Collaborator Author

Changes by Michael Bayer (@zzzeek):

  • added labels: orm

@sqlalchemy-bot
Copy link
Collaborator Author

Changes by Michael Bayer (@zzzeek):

  • set milestone to "1.x.xx"

@sqlalchemy-bot sqlalchemy-bot added bug Something isn't working orm low priority labels Nov 27, 2018
@sqlalchemy-bot sqlalchemy-bot added this to the 1.x.xx milestone Nov 27, 2018
@sqla-tester
Copy link
Collaborator

Mike Bayer has proposed a fix for this issue in the master branch:

Implement SynonymProperty.get_history() https://gerrit.sqlalchemy.org/1055

@zzzeek zzzeek modified the milestones: 1.x.xx, 1.3 Dec 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working low priority orm
Projects
None yet
Development

No branches or pull requests

3 participants