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

Misleading documentation using assert statement for validation #5596

Closed
wyfo opened this issue Sep 17, 2020 · 2 comments
Closed

Misleading documentation using assert statement for validation #5596

wyfo opened this issue Sep 17, 2020 · 2 comments
Assignees
Labels
documentation use case not really a feature or a bug; can be support for new DB features or user use cases not anticipated

Comments

@wyfo
Copy link

wyfo commented Sep 17, 2020

Validators documentation presents validators using assert statement to validate data. However, assert is a debug statement, which is disabled (and then validation using it) when run in optimized mode.

assert should only be used to ensure already trusted data, but I assume this is not the principal use case.
That's why the documentation should be updated to replace this statement by raising a ValueError instead. A note could be added to explain why assert is not the statement you should use in validation, excepted if you know what you are doing.

@wyfo wyfo added the requires triage New issue that requires categorization label Sep 17, 2020
@CaselIT CaselIT added documentation use case not really a feature or a bug; can be support for new DB features or user use cases not anticipated and removed requires triage New issue that requires categorization labels Sep 18, 2020
@CaselIT
Copy link
Member

CaselIT commented Sep 18, 2020

Hi,

Thanks for reporting this. If you want to provide a pull request, that would be great!

@jvanasco jvanasco self-assigned this Sep 8, 2021
@sqla-tester
Copy link
Collaborator

jonathan vanasco has proposed a fix for this issue in the master branch:

Fixes: #5596 https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/3071

sqlalchemy-bot pushed a commit that referenced this issue Sep 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation use case not really a feature or a bug; can be support for new DB features or user use cases not anticipated
Projects
None yet
Development

No branches or pull requests

4 participants