Skip to content

Conversation

gkevinzheng
Copy link
Contributor

Additional system tests for testing shim compatibility with the classic client interface.

Also fixes a bug where encountering a retriable error after another retriable error mid-stream raised an exception instead of retrying.

@gkevinzheng gkevinzheng requested review from a team as code owners October 14, 2025 18:50
@product-auto-label product-auto-label bot added the size: l Pull request size is large. label Oct 14, 2025
@product-auto-label product-auto-label bot added the api: bigtable Issues related to the googleapis/python-bigtable API. label Oct 14, 2025
@gkevinzheng gkevinzheng changed the base branch from main to v3_staging October 15, 2025 14:40
Copy link
Contributor

@daniel-sanche daniel-sanche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple comments for now, but I'll look at the tests in more detail soon

env_vars: {
key: "NOX_SESSION"
value: "system-3.12"
} No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pulled in main, so this shouldn't be necessary anymore

self._row_merger = _RowMerger(self._row_merger.last_seen_row_key)
self.response_iterator = self.read_method(retry_request)
self.response_iterator = self.read_method(retry_request, retry=self.retry)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why was this change needed?

class SelectiveMethodsErrorInjector(UnaryStreamClientInterceptor):
def __init__(self):
# As long as there are more items on this list, the items on the list
# are as follows:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume after the list is exhausted, it always behaves as normal?

data_table, rows_to_delete, row_keys, columns, CELL_VAL_READ_ROWS_RETRY
)

yield data_table
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this re-populating the table before each test function? Is that necessary?



def _populate_table(data_table, rows_to_delete, row_keys):
def _add_test_error_handler(retry):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a docstring could be helpful here

It looks like it overrides the on_error function to assert that backoff values are within expected bounds?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigtable Issues related to the googleapis/python-bigtable API. size: l Pull request size is large.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants