Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds concurrency testing for the write_table API by introducing a parameterized test that validates concurrent write operations with multiple threads.
- Adds a new test
test_concurrent_write_tablesthat uses threading to test concurrent writes - Changes the
table_filepathfixture scope from "module" to "function" to ensure test isolation
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| rerun_py/tests/e2e_redap_tests/test_table_write.py | Adds concurrent write test with threading support |
| rerun_py/tests/e2e_redap_tests/conftest.py | Changes fixture scope to function-level for better test isolation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ntjohnson1
left a comment
There was a problem hiding this comment.
Test looks reasonable to me as long as we assume write_table releases the GIL. The test doesn't ensure that the threads run concurrently. If the threads run sequentially this would still pass IIUC.
Approving if that is the desired scope, aince releasing the GIL should be supported by work in datafusion-python (that we both touched parts of).
This adds unit testing for write tables APIs to test concurrency.