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

feat: exclude implicit fields for sqlalchemy dto #2170

Merged
merged 3 commits into from
Sep 7, 2023

Conversation

abdulhaq-e
Copy link
Member

@abdulhaq-e abdulhaq-e commented Aug 15, 2023

Pull Request Checklist

  • New code has 100% test coverage
  • (If applicable) The prose documentation has been updated to reflect the changes introduced by this PR
  • (If applicable) The reference documentation has been updated to reflect the changes introduced by this PR

Description

SQLAlchemyDTO can now be configured using a separate config object. This can be set using both class inheritance and Annotated:

class MyModelDTO(SQLAlchemyDTO[MyModel]):
    config = SQLAlchemyDTOConfig()

or

MyModelDTO = SQLAlchemyDTO[Annotated[MyModel, SQLAlchemyDTOConfig()]]

The new configuration currently accepts a single attribute which is include_implicit_fields that has a default value of True. If set to to False, all implicitly mapped columns will be hidden from the DTO. If set to "hybrid-only", then hybrid properties will be shown but not other implicit columns. Finally, implicit columns that are marked with Mark.READ_ONLY or Mark.WRITE_ONLY will always be shown regardless of the value of include_implicit_fields.

Close Issue(s)

@abdulhaq-e abdulhaq-e marked this pull request as ready for review August 16, 2023 22:14
@abdulhaq-e abdulhaq-e requested review from a team as code owners August 16, 2023 22:14
Copy link
Member

@JacobCoffee JacobCoffee left a comment

Choose a reason for hiding this comment

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

small docs changes

litestar/dto/config.py Outdated Show resolved Hide resolved
litestar/dto/config.py Outdated Show resolved Hide resolved
@provinzkraut
Copy link
Member

@abdulhaq-e Please change the target branch to v2.1 and rebase from v2.1.

@abdulhaq-e abdulhaq-e changed the base branch from main to v2.1 August 18, 2023 13:21
@abdulhaq-e
Copy link
Member Author

@abdulhaq-e Please change the target branch to v2.1 and rebase from v2.1.

Done.

litestar/contrib/sqlalchemy/dto.py Outdated Show resolved Hide resolved
litestar/contrib/sqlalchemy/dto.py Outdated Show resolved Hide resolved
litestar/contrib/sqlalchemy/dto.py Outdated Show resolved Hide resolved
@provinzkraut
Copy link
Member

@cofin can you take a look at this one as well?

@cofin
Copy link
Member

cofin commented Aug 21, 2023

@cofin can you take a look at this one as well?

This one is now resolved. The fix is waiting to merge into main.

Copy link
Member

@provinzkraut provinzkraut left a comment

Choose a reason for hiding this comment

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

Okay, this looks good except for the minor comments I left!

litestar/contrib/sqlalchemy/dto.py Outdated Show resolved Hide resolved
litestar/contrib/sqlalchemy/dto.py Outdated Show resolved Hide resolved
litestar/dto/base_dto.py Outdated Show resolved Hide resolved
@github-actions
Copy link

Documentation preview will be available shortly at https://litestar-org.github.io/litestar-docs-preview/2170

use separate sqlalchemy dto config

support Annotated

allow overrides for hybrid properties and non-private Mark fields

Apply suggestions from code review

Update litestar/contrib/sqlalchemy/dto.py

Co-authored-by: Alc-Alc <45509143+Alc-Alc@users.noreply.github.com>

pythonic findWhere

docs update

use subclass implementation

typo

update lock file

use poetry.lock from main

Switch to private methods
@Goldziher Goldziher merged commit 6fe22e4 into litestar-org:main Sep 7, 2023
11 of 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.

None yet

6 participants