Skip to content

feat: add support for AddToCell in Data Client #1147

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

Merged
merged 12 commits into from
Jul 28, 2025

Conversation

axyjo
Copy link
Contributor

@axyjo axyjo commented Jul 3, 2025

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #1146 🦕

@axyjo axyjo requested review from a team as code owners July 3, 2025 14:25
@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: bigtable Issues related to the googleapis/python-bigtable API. labels Jul 3, 2025
@daniel-sanche
Copy link
Contributor

daniel-sanche commented Jul 10, 2025

Thanks for putting this together! Do you think you could also add a system test like this, to make sure it works as we expect end-to-end?

Edit: Actually, I ended up putting this together myself. You can copy it in: axyjo/python-bigtable@axyjo-data-client-add-to-cell...googleapis:python-bigtable:data-client-add-to-cell-system-tests

@daniel-sanche
Copy link
Contributor

daniel-sanche commented Jul 11, 2025

In case you miss my edit: I put together a system test for this. It would be great if you could merge that in here

@product-auto-label product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Jul 15, 2025
@axyjo
Copy link
Contributor Author

axyjo commented Jul 15, 2025

Thanks, @daniel-sanche + @mutianf! Just addressed your feedback + cherry-picked Daniel's changes.

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.

One more small change, then it looks good!

@axyjo axyjo requested a review from daniel-sanche July 16, 2025 12:16
daniel-sanche
daniel-sanche previously approved these changes Jul 16, 2025
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.

LGTM (after checks pass). Thanks for putting this together!

@daniel-sanche daniel-sanche added the kokoro:run Add this label to force Kokoro to re-run the tests. label Jul 18, 2025
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Jul 18, 2025
@daniel-sanche daniel-sanche added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 18, 2025
@daniel-sanche daniel-sanche self-requested a review July 18, 2025 20:37
daniel-sanche
daniel-sanche previously approved these changes Jul 18, 2025
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 18, 2025
@daniel-sanche daniel-sanche requested a review from a team July 18, 2025 20:37
mutianf
mutianf previously approved these changes Jul 18, 2025
@daniel-sanche
Copy link
Contributor

Sorry, one more change: It looks like some tests are breaking because of the new column family in the system tests. Can you apply this diff to fix the CI? (It also adds another unit test as well)

@axyjo axyjo dismissed stale reviews from mutianf and daniel-sanche via d33e798 July 25, 2025 02:12
@axyjo
Copy link
Contributor Author

axyjo commented Jul 25, 2025

@daniel-sanche apologies for the delay -- just pushed up those changes :)

@axyjo axyjo requested review from daniel-sanche and mutianf July 25, 2025 02:12
@daniel-sanche daniel-sanche added the kokoro:run Add this label to force Kokoro to re-run the tests. label Jul 25, 2025
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Jul 25, 2025
@daniel-sanche daniel-sanche added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 25, 2025
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 25, 2025
@axyjo
Copy link
Contributor Author

axyjo commented Jul 28, 2025

@daniel-sanche I think these Kokoro failures are unrelated to the changes, right?

@daniel-sanche daniel-sanche added the owlbot:run Add this label to trigger the Owlbot post processor. label Jul 28, 2025
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Jul 28, 2025
@daniel-sanche daniel-sanche merged commit 1a5b4b5 into googleapis:main Jul 28, 2025
27 of 31 checks passed
@daniel-sanche
Copy link
Contributor

Yes, we can ignore those sample failures

Just merged, thanks for all your work on this!

@axyjo
Copy link
Contributor Author

axyjo commented Jul 29, 2025

Thanks for your help w/ the system tests!

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.

Data client missing support for AddToCell
5 participants