graphql-python / graphql-python/graphql-core-legacy
Usage of Asserts Breaks Code
- 主要言語
- Python
- スター
- 371
- フォーク
- 175
- PR マージ指標
- 30日以内にマージされた PR はありません
説明
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.
コントリビューションガイド
このリポジトリのコントリビューションガイドは索引されていません
調査の方向性
issueで説明されている、引数検証にassertを使用している約100箇所をリポジトリ内で検索し、通常の実行時とPython -Oでの実行時の動作を比較します。影響を受ける各呼び出し箇所について想定されるエラーメッセージを確認し、最適化された実行でも既存のAssertionError互換性を変更せずに無効な入力を引き続き拒否することを確認します。
索引モデルが issue の本文から書いたものです。
評価
- 技術スタック
- graphql, python
- 領域
- backend-api-design
- issue の種類
- バグ
- 難易度
- 5/5
- 見積もり時間
- 1週間以上
- 活発さ
- 停滞
- 明瞭さ
- おおむね明確
- 初心者へのやさしさ
- 25/100