Repository navigation
[Claude] Produce Beam-specific representation of BQ resources, rework compat shims - #40359
jrmccluskey wants to merge 2 commits into
Conversation
|
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
89ae8b6 to
cf232f7
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #40359 +/- ##
=============================================
+ Coverage 56.16% 79.94% +23.77%
=============================================
Files 1124 488 -636
Lines 178336 81734 -96602
Branches 1489 0 -1489
=============================================
- Hits 100170 65339 -34831
+ Misses 75645 16395 -59250
+ Partials 2521 0 -2521
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Assigning reviewers: R: @damccorm for label python. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
| # | ||
|
|
||
| """Compatibility shims and legacy client emulation for BigQuery. | ||
| """Bridge for legacy apitools BigQuery message inputs. |
There was a problem hiding this comment.
I'm having a hard time orienting myself in this PR. Could you help me with a few things before I fully jump in:
- Could you give an overarching summary of what the PR is trying to accomplish? Is the idea basically that instead of emulating the old client, we convert the type to just use the new client (isn't this still basically emulating the client?)?
- Any recommendations on how to review this? Or are there ways to split it up? Should I review this file as a diff or just a replacement?
There was a problem hiding this comment.
There were a number of problems once I tried to actually move BQ interactions over to the new client while maintaining the old routing which led to a really tangled web of failing test cases and mismatched behavior, along with a lot of branches in the code based on the client being used. A second iteration of the idea (partially represented here) is to have Beam maintain its own representations of BQ resources that get passed to a specific backend at runtime (either the modern or legacy client) that converts the beam-specific implementation to the framework specific types as needed. Then the backend implementations either route to the new client or the old one.
Coming back to this particular approach after not looking at it for a few weeks, I'm not stoked with all of the scaffolding that goes into temporarily supporting multiple approaches when the end goal is to only support one at the end anyway. I think I'm waiving the white flag on the AI driven approach to this one, it's well past time to just write a doc instead.
Implements Beam-specific representations of BQ resource types to act as intermediaries between Beam and BigQuery types. Also reworks bigquery_compat.py to provide methods that convert between the legacy apitools types and these new type representations.
Part of #37261
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.