sqlmodel icon indicating copy to clipboard operation
sqlmodel copied to clipboard

πŸ› Support mixins

Open byrman opened this issue 2 years ago β€’ 22 comments

Add support for mixins to be used with SQLModel. See https://github.com/tiangolo/sqlmodel/issues/254.

byrman avatar Feb 28 '22 11:02 byrman

I thought I was already using mixins, when say this in our fastapi-users integration (using https://github.com/fastapi-users/fastapi-users-db-sqlmodel)

class User(SQLModelBaseUserDB, UserBase, table=True):

But am guessing that works because both those classes inherit SQLModel.

Just double checking: is it a good idea to add this to support mixins that are not SQLModel's? I guess yes.

antont avatar Feb 28 '22 12:02 antont

Would it be possible for you to test this with your project, @antont? It would be reassuring if your tests pass as well.

byrman avatar Feb 28 '22 14:02 byrman

Would it be possible for you to test this with your project, @antont? It would be reassuring if your tests pass as well.

Yes, I can do that either later today or else at some point tomorrow.

antont avatar Feb 28 '22 15:02 antont

Is there hope for a merge at all? I see a long list of useful PRs that haven't been merged and are outdated by now.

bolau avatar Mar 01 '22 07:03 bolau

Is there hope for a merge at all? I see a long list of useful PRs that haven't been merged and are outdated by now.

I just merge the PRs we need in our company fork (public). I guess others do the same. But yes there is hope, tiangolo seems to merge too and release when he has time, is just busy working on these on other fronts too, fastapi, pydantic etc.

SQLModel is quite small and was pretty solid when it was first released, I'd bet most of the PRs are not outdated as not much has changed.

antont avatar Mar 01 '22 08:03 antont

@byrman - works for me, so didn't break mixins that derive from SQLModel. makes sense too, didn't suspect problems either and the change seems logical too.

antont avatar Mar 01 '22 18:03 antont

Thanks for testing in the wild, @antont!

byrman avatar Mar 01 '22 20:03 byrman

Hope to see this merged soon πŸ™

amontalban avatar Apr 28 '22 12:04 amontalban

Until this is merged, here's an easy workaround:

class MyMixin():
    __config__ = None  # <---- THIS!
    # your code here

class MyModel(SQLModel, MyMixin, table=True):
    # your code here

bolau avatar Apr 28 '22 13:04 bolau

Unfortunately, this use case is not covered: https://github.com/tiangolo/sqlmodel/issues/330. Only methods and simple attributes appear to work.

byrman avatar May 05 '22 21:05 byrman

Thanks for testing these solutions.

@bolau I just tried your suggested approach and am having an issue where the introduction of __config__ = None is leading to an AttributeErrorwithinpydantic`. Is this something you've encountered before?

  File "/code/./library/module/utils.py", line 15, in <module>

    class ActiveRecord(SQLModel):

  File "/usr/local/lib/python3.9/site-packages/sqlmodel/main.py", line 277, in __new__

    new_cls = super().__new__(cls, name, bases, dict_used, **config_kwargs)

  File "/usr/local/lib/python3.9/site-packages/pydantic/main.py", line 292, in __new__

    cls.__try_update_forward_refs__()

  File "/usr/local/lib/python3.9/site-packages/pydantic/main.py", line 773, in __try_update_forward_refs__

    update_model_forward_refs(cls, cls.__fields__.values(), cls.__config__.json_encoders, {}, (NameError,))

AttributeError: 'NoneType' object has no attribute 'json_encoders'

AyrtonB avatar May 10 '22 16:05 AyrtonB

Codecov Report

Merging #256 (3c68587) into main (75ce455) will increase coverage by 0.00%. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #256   +/-   ##
=======================================
  Coverage   97.76%   97.77%           
=======================================
  Files         187      188    +1     
  Lines        6268     6280   +12     
=======================================
+ Hits         6128     6140   +12     
  Misses        140      140           
Impacted Files Coverage Ξ”
sqlmodel/main.py 84.92% <100.00%> (ΓΈ)
tests/test_mixin.py 100.00% <100.00%> (ΓΈ)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Aug 31 '22 15:08 codecov[bot]

πŸ“ Docs preview for commit 3c68587bab8c9ff940263a665173d04c3c3e0ae8 at: https://630f80b819e9472c1e2ab9ba--sqlmodel.netlify.app

github-actions[bot] avatar Aug 31 '22 15:08 github-actions[bot]

Sorry, no. Maybe it's not a good workaround after all ;-) Hope that this gets merged sometime soon...

bolau avatar Sep 27 '22 11:09 bolau

#256 fixes Mixin usage with https://github.com/absent1706/sqlalchemy-mixins ;)

deajan avatar Nov 22 '22 13:11 deajan

πŸ“ Docs preview for commit 6f0c4bb80281716b57b53dbfcfff55565a56d446 at: https://639ce01f2b35ad0062e48357--sqlmodel.netlify.app

github-actions[bot] avatar Dec 16 '22 21:12 github-actions[bot]

Well... up ! Any reason mixin support cannot be merged ?

deajan avatar Dec 27 '22 13:12 deajan

@byrman will relationship fields in mixins work with this?

thedamnedrhino avatar Jan 31 '23 18:01 thedamnedrhino

@byrman will relationship fields in mixins work with this?

I'm afraid not, take a look at https://github.com/tiangolo/sqlmodel/issues/330.

byrman avatar Feb 01 '23 09:02 byrman

Until this is merged, here's an easy workaround:

class MyMixin():
    __config__ = None  # <---- THIS!
    # your code here

class MyModel(SQLModel, MyMixin, table=True):
    # your code here

This worked well for me. I don't see official support for mixins in the documentation yet.

russellromney avatar Sep 01 '23 08:09 russellromney

@tiangolo Sorry to bother, but any reason this actually doesn't get pulled ?

deajan avatar Sep 01 '23 11:09 deajan

The __config__ = None workaround works for me, but causes this mypy error (as expected):

error: Definition of "__config__" in base class "BaseModel" is incompatible with definition in base class "MyMixin"

jpmckinney avatar Nov 28 '23 14:11 jpmckinney