chore(storage): integrate Compose in new interface #6414
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
chore: sync main to storage-refactor
chore: sync main to storage-refactor
* chore: release main (googleapis#6351) * test(profiler): compile busybench before running backoff test (googleapis#6375) * chore(bigquery/storage/managedwriter): augment test logging (googleapis#6373) * chore(storage): RewriteObject implementation (googleapis#6313) * chore(storage): RewriteObject implementation * address feedback * refactor source/destination object types * address feedback * address feedback * fix test * chore(main): release storage 1.24.0 (googleapis#6336) Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Chris Cotter <[email protected]> * chore(internal/gapicgen): update microgen v0.31.2 (googleapis#6383) Only includes fixes to regapic generation. * test(bigquery/storage/managedwriter): relax error checking (googleapis#6385) When a user issues a large request, the response from the backend is a bare "InvalidArgument". This PR removes additional validation on information that is only attached when interrogating the backend from a known client; it's stripped in the normal case. Internal issue 239740070 was created to address the unactionable nature of the response. Fixes: googleapis#6361 * feat(firestore): adds Bulkwriter support to Firestore client (googleapis#5946) * feat: adds Bulkwriter support to Firestore client * test(storage): unflake TestIntegration_ACL (googleapis#6392) Few minor changes to make sure potentially flaky and/or eventually consistent operations for ACLs are retried appropriately. Fixes googleapis#6379 Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Amarin (Um) Phaosawasdi <[email protected]> Co-authored-by: shollyman <[email protected]> Co-authored-by: Noah Dietz <[email protected]> Co-authored-by: Chris Cotter <[email protected]> Co-authored-by: Eric Schmidt <[email protected]>
Accidentally used squash. Reverts googleapis#6386
chore: sync main to storage-refactor
chore: sync main to storage-refactor
Change production code paths for Composer to use the new transport agnostic interface. Required a few small changes to make tests pass: * Add initialization of tc to NewClient * Add new useGRPC flag to remove switches on tc in Reader/Writer * A unit test checks that setting Generation with composer causes a failure. Currently this is enforced by applyConds. I added a separate validation for this instead, but I could do a pass- through of the Generation through the interface -- it's just confusing looking because setting Generation on a new object is never valid.
product-auto-label
bot
added
size: m
noahdietz
approved these changes
Jul 25, 2022
storage/copy.go
Outdated
| call := c.dst.c.raw.Objects.Compose(c.dst.bucket, c.dst.object, req).Context(ctx) | ||
| if err := applyConds("ComposeFrom destination", c.dst.gen, c.dst.conds, call); err != nil { | ||
| return nil, err | ||
| // TODO: factor this out to a function? |
There was a problem hiding this comment.
Is the definition of idempotency here a generalized one? Or is it specific to Copy/Compose?
There was a problem hiding this comment.
It's specific to copy/compose. However was thinking I could add a helper func to create the storage opts just to eliminate a few of these lines of boilerplate which will be in every func:
func makeStorageOpts(idempotent bool, retry *retryConfig, userProject string) []storageOption {...}
Maybe this defeats the point of the options interface though?
There was a problem hiding this comment.
Maybe this defeats the point of the options interface though?
Perhaps...let's just keep the TODO and evaluate as we go?
There was a problem hiding this comment.
I'll be adding this in a separate PR.
noahdietz
added
the
kokoro:force-run
kokoro-team
removed
the
kokoro:force-run
I reran kokoro and it actually executed this time. Just need to fix a lint error |
noahdietz
force-pushed
the
storage-refactor
branch
from
4e12635 to
2971037
Compare
tritone
added
the
kokoro:force-run
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.
Change production code paths for Composer to use the new transport
agnostic interface.
Required a few setup steps to make tests pass, which will be needed for
other integrations too:
Also a few cleanups:
Also, a unit test checks that setting Generation with composer causes
a failure. Currently this is enforced by applyConds. I added
a separate validation for this instead, but I could do a pass-
through of the Generation through the interface -- it's just
confusing looking because setting Generation on a new object
is never valid.