-
Notifications
You must be signed in to change notification settings - Fork 17
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
[Architecture] Switch over to a common impl with separate drivers #433
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Raised this as a draft to allow conversation to first happen about the high level approach before we get into the nuts and bolts of this change. |
mhutchinson
force-pushed
the
babyDriver
branch
2 times, most recently
from
January 6, 2025 17:09
fc2700d
to
8c121ce
Compare
The functionality is the same, but now all personalities have a common wrapper around a driver instead of having separate implementations directly in their hand. This is a bit more convoluted than seems necessary, but the layers of indirection allow us to properly hide the implementation methods because they're only available via a class that is internal. The big advantage of the main API being this shape is that Tessera now "feels" like a single API, with drivers to bind it to different environments. This is more similar to canonical usage of sql/db package, which many go developers are familiar with. Another advantage realized in this change is that deduplication is now folded into the core Appender object, instead of functionally floating around alongside it and needing to be handled by the personalities.
Unclear why this happens only when I use a go workspace, but there's some underlying problem here. googleapis/google-cloud-go#11283
mhutchinson
force-pushed
the
babyDriver
branch
from
January 8, 2025 11:49
8c121ce
to
e254a02
Compare
Makes diff smaller again
Also return error instead of panicking
AlCutter
approved these changes
Jan 8, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The functionality is the same, but now all personalities have a common wrapper around a driver instead of having separate implementations directly in their hand.
The big advantage of the main API being this shape is that Tessera now "feels" like a single API, with drivers to bind it to different environments. This is more similar to canonical usage of sql/db package, which many go developers are familiar with.
Another advantage realized in this change is that deduplication is now folded into the core Appender object, instead of functionally floating around alongside it and needing to be handled by the personalities.