Skip to content

Fix builder not copying hybrid_property.expr and other props to the version class#147

Merged
AbdealiLoKo merged 1 commit intocorridor:mainfrom
maximvlah:fix-hybrid-properties
Nov 18, 2025
Merged

Fix builder not copying hybrid_property.expr and other props to the version class#147
AbdealiLoKo merged 1 commit intocorridor:mainfrom
maximvlah:fix-hybrid-properties

Conversation

@maximvlah
Copy link

Found and fixed the bug in Builder.create_hybrid_properties. It was only copying the python hybrid property function to the corresponding version class but not the SQL equivalent, thus we would get the error below:

self = <class 'sqlalchemy_history.model_builder.ArticleVersion'>

    @sa.ext.hybrid.hybrid_property
    def author_name(self):
>       return self.author.name
E       AttributeError: 'property' object has no attribute 'name'

tests/test_hybrid_property.py:37: AttributeError
================================================================================================= short test summary info ==================================================================================================
FAILED tests/test_hybrid_property.py::TestHybridProperty::test_version_class_hybrid_property_in_sql_expression - AttributeError: 'property' object has no attribute 'name'

@maximvlah maximvlah changed the title Fix builder not copying hybrid_property.expr and other proprs to the version class Fix builder not copying hybrid_property.expr and other props to the version class Mar 28, 2025
@abulvenz
Copy link

Thanks for fixing it, we ran into problems here as well!

@aditya051293 Would you mind having a look?

Copy link
Collaborator

@ashish16052 ashish16052 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maximvlah Looks good to me. Can you please create a tracking issue and reference it in your commits for better traceability?

@AbdealiLoKo AbdealiLoKo force-pushed the fix-hybrid-properties branch from d039a6b to b9c5e86 Compare November 18, 2025 22:40
When creating the hybrid-prop in the version class, we just copied the
getter. Create the hybrid-prop properly
@AbdealiLoKo AbdealiLoKo force-pushed the fix-hybrid-properties branch from b9c5e86 to 7546991 Compare November 18, 2025 22:41
@AbdealiLoKo AbdealiLoKo merged commit 19cd3a0 into corridor:main Nov 18, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants