graphene-sqlalchemy icon indicating copy to clipboard operation
graphene-sqlalchemy copied to clipboard

Automatic sort argument for SQLAlchemyInterface

Open gbunkoczi opened this issue 1 year ago • 4 comments

When using SQLAlchemyConnectionField with a SQLAlchemyInterface to get a polymorphic connection, one has to set the sort argument to None to disable automatic sort enum generation. However, the type check in register_sort_enum is too strict, and being an instance of SQLAlchemyBase seems to be sufficient for the code to work.

Tests run with Python 3.8 and 3.12.

gbunkoczi avatar Jan 11 '24 16:01 gbunkoczi

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (9c2bc84) 94.74% compared to head (f5e6d32) 94.74%.

:exclamation: Current head f5e6d32 differs from pull request most recent head 3ade1e6. Consider uploading reports for the commit 3ade1e6 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #400   +/-   ##
=======================================
  Coverage   94.74%   94.74%           
=======================================
  Files          10       10           
  Lines        1333     1333           
=======================================
  Hits         1263     1263           
  Misses         70       70           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jan 11 '24 20:01 codecov[bot]

Apologies for the delay. I put some tests in place.

Somewhat tangentially, there seems to be more cases of "issubclass(obj_type, SQLAlchemyObjectType)" tests in the codebase, but since these were not triggered so far, I have not changed them. In addition, "- obj_type : SQLAlchemyObjectType" appears multiple times in function docstrings where " -obj_type: SQLALchemyBase" would be more appropriate, e.g. https://github.com/graphql-python/graphene-sqlalchemy/blob/9c2bc8468f4c88fee2b2f6fd2c0b8725fa9ccee3/graphene_sqlalchemy/enums.py#L94 https://github.com/graphql-python/graphene-sqlalchemy/blob/9c2bc8468f4c88fee2b2f6fd2c0b8725fa9ccee3/graphene_sqlalchemy/enums.py#L166

(these call registry.register_sort_enum, which has been switched to test SQLALchemyBase). I am happy to correct these as part of this PR if this would help.

gbunkoczi avatar Jan 23 '24 11:01 gbunkoczi

@gbunkoczi Awesome! if you have time, please feel free to go ahead. Thanks for your effort! 😊

erikwrede avatar Jan 23 '24 13:01 erikwrede

The checks have all been changed. I tried each call with a SQLAlchemyInterface instance, and all worked as expected. The docstrings were also updated when appropriate.

gbunkoczi avatar Feb 08 '24 14:02 gbunkoczi