graphql-python / graphql-python/graphql-core-legacy
Usage of Asserts Breaks Code
- Lingua principale
- Python
- Stelle
- 372
- Fork
- 175
- Metriche di merge delle PR
- Nessuna PR unita negli ultimi 30g
Descrizione
This project uses asserts in a lot of places (I found about 100 cases) where they should **not** be used. They're being used to validate arguments to functions, where the normal Python way would be to use a `TypeError` or a `ValueError`. Assertions are essentially debug code, not code for validating arguments.
Any usage of optimize mode or Python (`-O` or `-OO`, or `PYTHONOPTIMIZE`) strips out assertions, making this code useless. That's very problematic if you're generating schema dynamically, as you won't get errors and this library will generate an invalid GraphQL schema.
At this point it's probably too late to change the uses of assert to a proper `TypeError` or `ValueError`, without a major version bump as dependent code in other projects may be catching `AssertionError`.
I'm proposing all instances of `assert foo, "Error"` be replaced with:
```python
if not foo:
raise AssertionError("Error") # Should be a TypeError
```
This will at least make the code work in optimize modes where assertions are stripped.
I'll gladly make a PR doing so, but it's hard to tell if this project is still actively developed.
Guida per i contributori
Nessuna guida per i contributori indicizzata per questo repository
Direzione di ricerca
Search the repository for the roughly 100 argument-validation uses of assert described in the issue, then compare their behavior under normal and Python -O execution. Review each affected call site for its expected error message and confirm that optimized execution still rejects invalid inputs without changing the existing AssertionError compatibility.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Valutazione
- Stack tecnologico
- graphql, python
- Ambito
- backend-api-design
- Tipo di issue
- Bug
- Difficoltà
- 5/5
- Tempo stimato
- Più di una settimana
- Stato di attività
- Ferma
- Chiarezza
- Abbastanza chiara
- Idoneità per principianti
- 25/100