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: support resource_tags for table #2093

Merged
merged 13 commits into from
Jan 21, 2025

Conversation

hrkh
Copy link
Contributor

@hrkh hrkh commented Dec 21, 2024

  • 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 #2089 🦕

@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: bigquery Issues related to the googleapis/python-bigquery API. labels Dec 21, 2024
@hrkh hrkh force-pushed the feature/resource-tags branch from 732426d to e783909 Compare December 21, 2024 04:30
@hrkh hrkh changed the title Support resource_tags for Table feat: support resource_tags for table Dec 21, 2024
@hrkh hrkh marked this pull request as ready for review December 21, 2024 05:16
@hrkh hrkh requested review from a team as code owners December 21, 2024 05:16
@hrkh hrkh requested a review from PhongChuong December 21, 2024 05:16
@hrkh hrkh marked this pull request as draft December 21, 2024 06:42
@hrkh hrkh marked this pull request as ready for review December 21, 2024 10:58
@Linchin
Copy link
Contributor

Linchin commented Jan 3, 2025

Sorry, this seems to be a duplicate of #2090.

@Linchin Linchin closed this Jan 3, 2025
@hrkh
Copy link
Contributor Author

hrkh commented Jan 6, 2025

Hi @Linchin ,
Thank you for your feedback.

This PR was closed as a duplicate of #2090, but I’d like to clarify that #2090 addresses the resource tags for datasets, while this PR focuses on the resource tags for tables.

Could you please reopen this PR for further review? Let me know if additional clarification is needed.

@Linchin Linchin reopened this Jan 6, 2025
@Linchin
Copy link
Contributor

Linchin commented Jan 6, 2025

Thank you @hrkh for clarifying! I have reopened and will review this PR.

tests/unit/test_client.py Outdated Show resolved Hide resolved
@Linchin Linchin added kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Jan 6, 2025
@yoshi-kokoro yoshi-kokoro removed kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Jan 6, 2025
@Linchin Linchin added kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Jan 16, 2025
@yoshi-kokoro yoshi-kokoro removed kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Jan 16, 2025
@Linchin Linchin added kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Jan 16, 2025
@yoshi-kokoro yoshi-kokoro removed kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Jan 16, 2025
@Linchin
Copy link
Contributor

Linchin commented Jan 17, 2025

Thank you @hrkh for your contribution! It seems we just have the system test with python 3.12 failing, and it was caused by the same resource tag having been created by the python 3.8 system test. We encountered the exact same issue in #2090, and it was resolved by prefixing a randomly generated string to the tags (as in this commit). Could you see if you could implement this, so we can get this PR across the line? Cheers.

@hrkh
Copy link
Contributor Author

hrkh commented Jan 18, 2025

Thank you @Linchin for your detailed feedback and guidance!

I've updated the PR to append randomly generated strings as suffixes to the tag keys, following the approach used in the referenced commit. This should resolve the test failures with Python 3.12.

Please let me know if any further adjustments are needed.

@Linchin Linchin added kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Jan 21, 2025
@Linchin Linchin added kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. and removed kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Jan 21, 2025
@yoshi-kokoro yoshi-kokoro removed kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Jan 21, 2025
@Linchin Linchin added kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Jan 21, 2025
@yoshi-kokoro yoshi-kokoro removed kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Jan 21, 2025
Copy link
Contributor

@Linchin Linchin left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

@Linchin Linchin merged commit d4070ca into googleapis:main Jan 21, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigquery Issues related to the googleapis/python-bigquery API. size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support resource_tags for Table
3 participants