cleanup(storage)!: session-less resumable uploads #8806

coryan · 2022-04-26T20:41:42Z

Fixes #8621. Fixes #7282. Fixes #6713. Fixes #7880

This change is

google-cloud-cpp-bot · 2022-04-26T20:45:08Z

Google Cloud Build Logs
For commit: 141c67d2b32148291d03a999b328d84acd5f5349

Public Access Build Logs
Cloud Build Console (requires auth)
Kokoro Logs (requires auth)

ℹ️ NOTE: Kokoro logs are linked from "Details" below.

codecov · 2022-04-26T21:04:20Z

Codecov Report

Merging #8806 (c406c5f) into main (039f9e4) will decrease coverage by 1.76%.
The diff coverage is 98.09%.

@@            Coverage Diff             @@
##             main    #8806      +/-   ##
==========================================
- Coverage   93.35%   91.58%   -1.77%     
==========================================
  Files        1470     1132     -338     
  Lines      125324    89435   -35889     
==========================================
- Hits       117001    81913   -35088     
+ Misses       8323     7522     -801

Impacted Files	Coverage Δ
google/cloud/storage/internal/curl_client.h	`100.00% <ø> (ø)`
google/cloud/storage/internal/grpc_client.cc	`80.90% <ø> (-10.07%)`	⬇️
google/cloud/storage/internal/grpc_client.h	`100.00% <ø> (ø)`
google/cloud/storage/internal/hybrid_client.h	`100.00% <ø> (ø)`
google/cloud/storage/internal/raw_client.h	`100.00% <ø> (ø)`
google/cloud/storage/internal/rest_client.h	`100.00% <ø> (+50.00%)`	⬆️
google/cloud/storage/testing/mock_client.h	`90.62% <ø> (-6.68%)`	⬇️
google/cloud/storage/internal/hybrid_client.cc	`86.84% <33.33%> (+1.49%)`	⬆️
...ud/storage/internal/object_write_streambuf_test.cc	`99.24% <98.61%> (-0.76%)`	⬇️
google/cloud/storage/client.cc	`86.07% <100.00%> (+0.13%)`	⬆️
... and 374 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 039f9e4...c406c5f. Read the comment docs.

google-cloud-cpp-bot · 2022-04-26T22:03:03Z

Google Cloud Build Logs
For commit: c2dd610af5ccd97969daf3424f436fe6da365fdf

Public Access Build Logs
Cloud Build Console (requires auth)
Kokoro Logs (requires auth)

ℹ️ NOTE: Kokoro logs are linked from "Details" below.

google-cloud-cpp-bot · 2022-04-26T23:53:34Z

Google Cloud Build Logs
For commit: 2efc5d426c74d2ece2bd9f3e2d4cd2764a40bca9

Public Access Build Logs
Cloud Build Console (requires auth)
Kokoro Logs (requires auth)

ℹ️ NOTE: Kokoro logs are linked from "Details" below.

google-cloud-cpp-bot · 2022-04-27T18:37:36Z

Google Cloud Build Logs
For commit: 674a8650a5dd1ce7b1f0edcd7a06f5c01e8b948e

Public Access Build Logs
Cloud Build Console (requires auth)
Kokoro Logs (requires auth)

ℹ️ NOTE: Kokoro logs are linked from "Details" below.

google-cloud-cpp-bot · 2022-04-27T19:09:43Z

Google Cloud Build Logs
For commit: d1ce2fc620816c78475e8705a252577ad0133134

Public Access Build Logs
Cloud Build Console (requires auth)
Kokoro Logs (requires auth)

ℹ️ NOTE: Kokoro logs are linked from "Details" below.

google-cloud-cpp-bot · 2022-04-28T15:34:45Z

Google Cloud Build Logs
For commit: 8ad959a471c3e25fa16e77a11a58652356d26927

Public Access Build Logs
Cloud Build Console (requires auth)
Kokoro Logs (requires auth)

ℹ️ NOTE: Kokoro logs are linked from "Details" below.

google-cloud-cpp-bot · 2022-04-28T16:02:52Z

Google Cloud Build Logs
For commit: 5e0cc81c94a297626f238f89bdd964dfd06166e8

Public Access Build Logs
Cloud Build Console (requires auth)
Kokoro Logs (requires auth)

ℹ️ NOTE: Kokoro logs are linked from "Details" below.

coryan · 2022-04-28T16:19:02Z

/FYI: @Jseph

google-cloud-cpp-bot · 2022-04-28T22:14:51Z

Google Cloud Build Logs
For commit: 29652e063fe3753517b581529719971ac4fb08a5

Public Access Build Logs
Cloud Build Console (requires auth)
Kokoro Logs (requires auth)

ℹ️ NOTE: Kokoro logs are linked from "Details" below.

google-cloud-cpp-bot · 2022-04-28T23:08:11Z

Google Cloud Build Logs
For commit: 042f898e8d19c5b0adcc1f59480fc4923a56ebe4

Public Access Build Logs
Cloud Build Console (requires auth)
Kokoro Logs (requires auth)

ℹ️ NOTE: Kokoro logs are linked from "Details" below.

dbolduc

nits only, most of which predated the PR

dbolduc · 2022-04-28T20:46:25Z

google/cloud/storage/internal/object_write_streambuf_test.cc

@@ -125,30 +112,30 @@ TEST(ObjectWriteStreambufTest, SmallStream) {
 /// @test Verify that uploading a stream which ends on a upload chunk quantum


nit: s/a upload/an upload/

dbolduc · 2022-04-29T10:46:34Z

google/cloud/storage/internal/object_write_streambuf_test.cc

-/// expected byte jumps.
-TEST(ObjectWriteStreambufTest, NextExpectedByteJumpsAhead) {
-  auto mock = absl::make_unique<testing::MockResumableUploadSession>();
+/// @test verify that the upload steam transitions to a bad state if the


nit: s/steam/stream/

dbolduc · 2022-04-29T10:46:49Z

google/cloud/storage/internal/object_write_streambuf_test.cc

@@ -341,25 +314,22 @@ TEST(ObjectWriteStreambufTest, NextExpectedByteJumpsAhead) {

 /// @test verify that the upload steam transitions to a bad state if the next


nit: s/steam/stream/

dbolduc · 2022-04-29T10:47:25Z

google/cloud/storage/internal/object_write_streambuf_test.cc

@@ -369,21 +339,18 @@ TEST(ObjectWriteStreambufTest, NextExpectedByteDecreases) {
 /// @test verify that the upload steam transitions to a bad state on a partial


nit: s/steam/stream/

dbolduc · 2022-04-29T11:01:28Z

google/cloud/storage/internal/object_write_streambuf_test.cc

-  std::string const payload_2("trailer");
-  size_t next_byte = 0;
+  std::string const p0(3 * quantum, '*');
+  std::string const p1("trailer");
  {


nit: are the braces necessary?

dbolduc · 2022-04-29T11:07:19Z

google/cloud/storage/internal/object_write_streambuf_test.cc

        // When using X-Upload-Content-Length GCS finalizes the upload when
        // enough data is sent, regardless of whether we use UploadChunk() or
        // UploadFinalChunk(). Furthermore, the response does not have a
        // committed size.


the bit about UploadFinalChunk() is stale

dbolduc · 2022-04-29T11:39:48Z

google/cloud/storage/client.cc

    error_stream.setstate(std::ios::badbit | std::ios::eofbit);
    error_stream.Close();
    return error_stream;
  }
  auto const buffer_size = request.GetOption<UploadBufferSize>().value_or(
      raw_client_->client_options().upload_buffer_size());
  return ObjectWriteStream(absl::make_unique<internal::ObjectWriteStreambuf>(
-      std::move(create->session), std::move(create->state), buffer_size,
+      raw_client_, request, std::move(std::move(response->upload_id)),


I do not think std::move(std::move(x)) is a thing.

dbolduc · 2022-04-29T11:55:04Z

google/cloud/storage/examples/storage_client_mock_samples.cc

+  EXPECT_CALL(*mock, UploadChunk)
+      .WillOnce(Return(
+          QueryResumableUploadResponse{/*committed_size=*/absl::nullopt,
+                                       /*object_metadata=*/expected_metadata}));



When I said "the flow seems nicer now", that was an understatement. It is way better.

dbolduc · 2022-04-29T12:13:57Z

google/cloud/storage/parallel_uploads_test.cc

+    using ::testing::AtMost;
+    EXPECT_CALL(*mock_, UploadChunk(Property(
+                            &UploadChunkRequest::upload_session_url, id)))
+        .Times(AtMost(1))


I guess this isn't a WillOnce(...) case?

For some reason, I thought .Times(0).WillRepeatedly(...) didn't work. Which makes me think that .Times(AtMost(1)) is really .Times(1).... but idk

Somehow I had convinced myself that this could be called one or zero times. Changed to .WillOnce().

google-cloud-cpp-bot · 2022-04-29T12:41:28Z

Google Cloud Build Logs
For commit: c406c5f642094498a48fc75775a25ade29570b47

Public Access Build Logs
Cloud Build Console (requires auth)
Kokoro Logs (requires auth)

ℹ️ NOTE: Kokoro logs are linked from "Details" below.

coryan force-pushed the cleanup-storage-remove-resumable-upload-session-pr-final branch from c2dd610 to 2efc5d4 Compare April 26, 2022 23:52

coryan force-pushed the cleanup-storage-remove-resumable-upload-session-pr-final branch from 2efc5d4 to 674a865 Compare April 27, 2022 18:28

coryan force-pushed the cleanup-storage-remove-resumable-upload-session-pr-final branch from 674a865 to d1ce2fc Compare April 27, 2022 19:08

coryan force-pushed the cleanup-storage-remove-resumable-upload-session-pr-final branch from d1ce2fc to 8ad959a Compare April 28, 2022 15:28

coryan marked this pull request as ready for review April 28, 2022 15:58

coryan requested a review from a team as a code owner April 28, 2022 15:58

coryan added 2 commits April 28, 2022 22:13

cleanup(storage)!: session-less resumable uploads

2c93c3e

Fix hybrid client

29652e0

coryan force-pushed the cleanup-storage-remove-resumable-upload-session-pr-final branch from 5e0cc81 to 29652e0 Compare April 28, 2022 22:13

I missed some conflicts

042f898

dbolduc approved these changes Apr 29, 2022

View reviewed changes

Address review comments

c406c5f

coryan enabled auto-merge (squash) April 29, 2022 12:43

coryan merged commit 301676d into googleapis:main Apr 29, 2022

coryan deleted the cleanup-storage-remove-resumable-upload-session-pr-final branch April 29, 2022 13:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cleanup(storage)!: session-less resumable uploads #8806

cleanup(storage)!: session-less resumable uploads #8806

coryan commented Apr 26, 2022 •

edited

Loading

google-cloud-cpp-bot commented Apr 26, 2022

codecov bot commented Apr 26, 2022 •

edited

Loading

google-cloud-cpp-bot commented Apr 26, 2022

google-cloud-cpp-bot commented Apr 26, 2022

google-cloud-cpp-bot commented Apr 27, 2022

google-cloud-cpp-bot commented Apr 27, 2022

google-cloud-cpp-bot commented Apr 28, 2022

google-cloud-cpp-bot commented Apr 28, 2022

coryan commented Apr 28, 2022

google-cloud-cpp-bot commented Apr 28, 2022

google-cloud-cpp-bot commented Apr 28, 2022

dbolduc left a comment

dbolduc Apr 28, 2022

coryan Apr 29, 2022

dbolduc Apr 29, 2022

coryan Apr 29, 2022

dbolduc Apr 29, 2022

coryan Apr 29, 2022

dbolduc Apr 29, 2022

coryan Apr 29, 2022

dbolduc Apr 29, 2022

coryan Apr 29, 2022

dbolduc Apr 29, 2022

coryan Apr 29, 2022

dbolduc Apr 29, 2022

coryan Apr 29, 2022

dbolduc Apr 29, 2022

coryan Apr 29, 2022

dbolduc Apr 29, 2022

coryan Apr 29, 2022

google-cloud-cpp-bot commented Apr 29, 2022

		@@ -125,30 +112,30 @@ TEST(ObjectWriteStreambufTest, SmallStream) {
		/// @test Verify that uploading a stream which ends on a upload chunk quantum

		@@ -341,25 +314,22 @@ TEST(ObjectWriteStreambufTest, NextExpectedByteJumpsAhead) {

		/// @test verify that the upload steam transitions to a bad state if the next

		@@ -369,21 +339,18 @@ TEST(ObjectWriteStreambufTest, NextExpectedByteDecreases) {
		/// @test verify that the upload steam transitions to a bad state on a partial

cleanup(storage)!: session-less resumable uploads #8806

cleanup(storage)!: session-less resumable uploads #8806

Conversation

coryan commented Apr 26, 2022 • edited Loading

google-cloud-cpp-bot commented Apr 26, 2022

codecov bot commented Apr 26, 2022 • edited Loading

Codecov Report

google-cloud-cpp-bot commented Apr 26, 2022

google-cloud-cpp-bot commented Apr 26, 2022

google-cloud-cpp-bot commented Apr 27, 2022

google-cloud-cpp-bot commented Apr 27, 2022

google-cloud-cpp-bot commented Apr 28, 2022

google-cloud-cpp-bot commented Apr 28, 2022

coryan commented Apr 28, 2022

google-cloud-cpp-bot commented Apr 28, 2022

google-cloud-cpp-bot commented Apr 28, 2022

dbolduc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

google-cloud-cpp-bot commented Apr 29, 2022

coryan commented Apr 26, 2022 •

edited

Loading

codecov bot commented Apr 26, 2022 •

edited

Loading