Skip to content
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

feat: surface retry param to Table.read_row api #982

Merged
merged 1 commit into from
Nov 7, 2024

Conversation

ayubun
Copy link
Contributor

@ayubun ayubun commented Jun 12, 2024

what ? ( ̄^ ̄ゞ

this pr surfaces a retry param for Table.read_row. this param defaults to DEFAULT_RETRY_READ_ROWS just as it does for Table.read_rows. it then passes forward into the Table.read_rows call

why ? („• ᴗ •„)

in the case where a caller may want to override the default retry param for the Table.read_row api, they currently must switch to using Table.read_rows and add their own code for pulling the first item out of the iterator and guarding against multi-row responses (which are functionalities that Table.read_row already provides)

in an ideal world, the Table.read_row helper method can accept and pass along the retry param so that clients don't need to write their own duplicate implementations wrapping Table.read_rows~

related issue ౨ৎ⋆˚。⋆

implements #941

Copy link

google-cla bot commented Jun 12, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link

conventional-commit-lint-gcf bot commented Jun 12, 2024

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

@product-auto-label product-auto-label bot added size: s Pull request size is small. api: bigtable Issues related to the googleapis/python-bigtable API. labels Jun 12, 2024
@ayubun ayubun force-pushed the ayu/add-retry-to-read_row branch from 299f94b to 2e83ad3 Compare June 12, 2024 11:24
@ayubun ayubun changed the title surface retry param to Table.read_row api feat: surface retry param to Table.read_row api Jun 12, 2024
@ayubun ayubun force-pushed the ayu/add-retry-to-read_row branch from 2e83ad3 to 8203d4f Compare June 12, 2024 11:53
@ayubun ayubun marked this pull request as ready for review June 12, 2024 11:56
@ayubun ayubun requested review from a team as code owners June 12, 2024 11:56
@ayubun
Copy link
Contributor Author

ayubun commented Oct 6, 2024

i wanted to bump this since it's been a while ^^; @rkaregar

@daniel-sanche daniel-sanche added the kokoro:run Add this label to force Kokoro to re-run the tests. label Oct 25, 2024
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Oct 25, 2024
@daniel-sanche daniel-sanche added the kokoro:run Add this label to force Kokoro to re-run the tests. label Oct 28, 2024
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Oct 28, 2024
@daniel-sanche daniel-sanche added the owlbot:run Add this label to trigger the Owlbot post processor. label Oct 28, 2024
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Oct 28, 2024
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.

There seems to be an issue in the owlbot test, unreleated to these changes. I'll merge this when that is resolved

@daniel-sanche daniel-sanche added kokoro:force-run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels Oct 28, 2024
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Oct 28, 2024
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 28, 2024
@daniel-sanche
Copy link
Contributor

Actually, it looks like it's failing on the noxfile customizations. These were stripped out in the upstream main owlbot.py file. So I think the issue is because the tests are running against your branch, which doesn't contain all the upstream changes.

Please merge in upstream's main, and then we should be good to go

@ayubun
Copy link
Contributor Author

ayubun commented Nov 5, 2024

@daniel-sanche done

@daniel-sanche daniel-sanche added the kokoro:run Add this label to force Kokoro to re-run the tests. label Nov 5, 2024
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Nov 5, 2024
@daniel-sanche
Copy link
Contributor

thanks! There's now a CI issue on the backend, but I will merge this when that is resolved

@daniel-sanche daniel-sanche added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 7, 2024
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 7, 2024
@daniel-sanche daniel-sanche added the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 7, 2024
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 7, 2024
@daniel-sanche daniel-sanche merged commit a8286d2 into googleapis:main Nov 7, 2024
22 of 30 checks passed
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: s Pull request size is small.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants