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

Add column property in the version table #301

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AbdealiLoKo
Copy link
Contributor

@AbdealiLoKo AbdealiLoKo commented Aug 31, 2022

polymorphic_on supports column_property() defined on the mapper.
But the version mapper does not have the column property making it fail even though the mapper_args are being copied into the version table.

This also makes it more natural for the user - because they can directly access column properties from the version table itself

This also copies the column_property() into the version tables

Relates to #297

Copy the column_property() defined in the parent model to the
version model
@AbdealiLoKo AbdealiLoKo force-pushed the ajk-colprop branch 3 times, most recently from c31f9f0 to 4348533 Compare August 31, 2022 14:20
@AbdealiLoKo
Copy link
Contributor Author

Right now - I have impleented this assuming all column properties should be copied - cause I thought that would be useful.
But if you think this is not a good idea, I can add the column property only if a column property is being used in the polymorphic_on

Also - @marksteward I was not sure whether this should be in builder.py or model_builder.py
I saw the mapper args were being added in model_builder.py but column-properties and relationships are being added in builder.py
I feel like this should be in model_builder.py

@AbdealiLoKo
Copy link
Contributor Author

@marksteward this one is blocking me from using continuum too

Does next-release mean next major or minor or patch release ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants