Raise TypeError on invalid query params.#2523
Conversation
tests/models/test_queryparams.py
Outdated
|
|
||
|
|
||
| def test_invalid_query_params(): | ||
| with pytest.raises(TypeError): |
There was a problem hiding this comment.
Seems nice to ensure that the formatted message is correct
| with pytest.raises(TypeError): | |
| with pytest.raises(TypeError, match=r"Expected str, int, float, bool, or None\. Got 'bytes'\."): |
There was a problem hiding this comment.
That's neat, thanks.
There was a problem hiding this comment.
Sorry I broke your line length :)
Co-authored-by: Michael Adkins <contact@zanie.dev>
| def test_invalid_query_params(): | ||
| with pytest.raises( | ||
| TypeError, match=r"Expected str, int, float, bool, or None\. Got 'bytes'\." | ||
| TypeError, match=r"Expected str, int, float, bool, or None. Got 'bytes'." |
There was a problem hiding this comment.
You don't need the r if you aren't escaping the periods — this is doing a re.search so technically the . will be a wildcard but 🤷♀️ it's not likely to cause you any problems. You could use re.escape if you really wanted to be correct.
|
@tomchristie this broke Prefect since we were passing UUID objects — that's fine but I think we should make sure this is listed on the changelog for 0.23.2 for visibility. |
|
Ooofff. Okay, more care needed in the future. |
|
That's entirely on me - rushing things that didn't need to be rushed. Do we want to roll this back in a 0.23.3, and leave it for the upcoming 0.24.0 update? (Or do we leave it as is?) |
|
It'd probably be safest to make this change in 0.24.0 instead since it changes the supported interface (I'll try to keep an eye out for this in future reviews), but I don't have strong feelings about rolling it back. |
This reverts commit 4cbf13e.
FYI - This also breaks datetime -> str conversion -- would suggest to keep datetime supported and adhering to RFC |
* Raise TypeError on invalid query params * Fix TypeError * Update tests/models/test_queryparams.py Co-authored-by: Michael Adkins <contact@zanie.dev> * Linting * Fix exception check Co-authored-by: Michael Adkins <contact@zanie.dev>
…code#2539) This reverts commit 4cbf13e.
Prompted by #2515. (Doesn't presume any particular resolution to that case.)
Closes #746 - we don't support bytes (or other unexpected types) as QueryParam values. Let's raise a clear error, rather than coercing an unexpected
strvalue.Before this change...
After this change...