refactor: data table bulk row replacement #123
Conversation
…ows function instead of client method
|
/run-integration-tests 0100c80 |
|
@gemini-cli /review |
|
🤖 Hi @mihirvala-crestdata, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
📋 Review Summary
This pull request refactors the replace_data_table_rows function to improve row size calculation and batching logic. The changes simplify the implementation by removing an unused method and leveraging the existing create_data_table_rows function for handling remaining rows.
🔍 General Feedback
- The refactoring is a good improvement, but it introduces a critical bug that will cause a
NameErrorat runtime. - The logging can be improved by using the
loggingmodule instead ofprint()statements, which is more consistent with the project's style. - The tests have been updated to reflect the refactoring, which is great. However, they might not be catching the introduced bug due to mocking. It's worth reviewing the tests to ensure they cover this case.
… message. chore: updated version and added changelog
|
@gemini-cli /review |
|
🤖 Hi @mihirvala-crestdata, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
There was a problem hiding this comment.
📋 Review Summary
This pull request refactors the replace_data_table_rows function to improve handling of large row counts by splitting the operation into a bulkReplace for the initial batch and bulkCreate for the remaining rows. The changes simplify the batching logic by leveraging the existing create_data_table_rows function.
🔍 General Feedback
- The refactoring is a good improvement, making the code cleaner and more robust for large datasets.
- The test cases have been updated effectively to cover the new logic, including size-based batching and chunking.
|
/run-integration-tests 4672bb3 |
…ace_data_table_rows
What
_create_row_batchesmethod fromdata_table.pyreplace_data_table_rowsindata_table.pyreplace_data_table_rows