-
Notifications
You must be signed in to change notification settings - Fork 169
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
Treat warning as error in CI/Dev #973
Conversation
Hi @ndrluis - thank you for turning the issue in #971 into these strategic items so quickly. I'm +1 for re-enabling this flag as it will help us avoid issues like you've reported. Testing this out in my dev environment, I see that other tests would also end up breaking with "expected" UserWarnings. And example is:
Some of our tests like: tests/catalog/test_sql.py::test_write_pyarrow_schema would fail in this case. One suggestion is to update this test to use |
@ndrluis shall we rebase this branch with the fix you issued in the other PR #972 ?
|
e8c1fdd
to
288e4ec
Compare
@sungwy Done! |
Thank you! Running the CI now |
This will help us avoid propagating warnings to our users, as occurred in apache#971.
288e4ec
to
4643f2f
Compare
4643f2f
to
b146ce6
Compare
Just to clarify the test changes:
These are filtering third-party warnings. I changed the table overwrite to append when it makes sense to avoid the "Delete operation did not match any records" error, and maintained the overwrite in an empty table or a delete operation where there is nothing to delete, as it makes sense. Additionally, the "Merge on read is not yet supported, falling back to copy-on-write" is an expected warning in the test context. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a fantastic contribution to the repository @ndrluis - thank you very much for putting this in and fixing up the tests 🚀
I'll leave this PR open for comments, and merge it in later today or tomorrow
This will help us avoid propagating warnings to our users, as occurred in #971.