From 5558855fe8026dd30e3a94411aef5b22c5d62cdc Mon Sep 17 00:00:00 2001 From: Cody Oss <6331106+codyoss@users.noreply.github.com> Date: Fri, 13 Aug 2021 10:36:25 -0600 Subject: [PATCH 1/4] docs(internal/carver): update commands and post tagging steps (#4613) --- internal/carver/cmd/main.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/internal/carver/cmd/main.go b/internal/carver/cmd/main.go index 12052bf0ee8..284a80827a0 100644 --- a/internal/carver/cmd/main.go +++ b/internal/carver/cmd/main.go @@ -365,10 +365,11 @@ func (c *carver) GitCommit() error { } } log.Println("Successfully carved out module. Please run the following commands after your change is merged:") - fmt.Fprintf(os.Stdout, "git tag -a %s \n", c.rootMod.Tag()) - fmt.Fprintf(os.Stdout, "git tag -a %s \n", c.childMod.Tag()) - fmt.Fprintf(os.Stdout, "git push origin ref/tags/%s\n", c.rootMod.Tag()) - fmt.Fprintf(os.Stdout, "git push origin ref/tags/%s\n", c.childMod.Tag()) + fmt.Fprintf(os.Stdout, "git tag %s \n", c.rootMod.Tag()) + fmt.Fprintf(os.Stdout, "git tag %s \n", c.childMod.Tag()) + fmt.Fprintf(os.Stdout, "git push origin refs/tags/%s\n", c.rootMod.Tag()) + fmt.Fprintf(os.Stdout, "git push origin refs/tags/%s\n", c.childMod.Tag()) + fmt.Fprintf(os.Stdout, "Once tags have propagated open a new PR tidying the new child mods go.sum entries.\n") return nil } From 27fc78456fb251652bdf5cdb493734a7e1e643e1 Mon Sep 17 00:00:00 2001 From: Brenna N Epp Date: Fri, 13 Aug 2021 12:03:52 -0500 Subject: [PATCH 2/4] fix(storage): remove unnecessary variable (#4608) Variable seems to be duplicating information which led to a bug. --- storage/storage.go | 3 --- storage/writer.go | 3 --- 2 files changed, 6 deletions(-) diff --git a/storage/storage.go b/storage/storage.go index 1b0610e380a..82048923cd5 100644 --- a/storage/storage.go +++ b/storage/storage.go @@ -90,8 +90,6 @@ type Client struct { raw *raw.Service // Scheme describes the scheme under the current host. scheme string - // EnvHost is the host set on the STORAGE_EMULATOR_HOST variable. - envHost string // ReadHost is the default host used on the reader. readHost string } @@ -152,7 +150,6 @@ func NewClient(ctx context.Context, opts ...option.ClientOption) (*Client, error hc: hc, raw: rawService, scheme: scheme, - envHost: host, readHost: readHost, }, nil } diff --git a/storage/writer.go b/storage/writer.go index 345f8bdb4c9..e468d5d8e87 100644 --- a/storage/writer.go +++ b/storage/writer.go @@ -125,9 +125,6 @@ func (w *Writer) open() error { if w.MD5 != nil { rawObj.Md5Hash = base64.StdEncoding.EncodeToString(w.MD5) } - if w.o.c.envHost != "" { - w.o.c.raw.BasePath = fmt.Sprintf("%s://%s", w.o.c.scheme, w.o.c.envHost) - } call := w.o.c.raw.Objects.Insert(w.o.bucket, rawObj). Media(pr, mediaOpts...). Projection("full"). From 8cfcf53d03b9b442e7f0bc1c1b20c791e31c07b0 Mon Sep 17 00:00:00 2001 From: Alex Hong <9397363+hongalex@users.noreply.github.com> Date: Fri, 13 Aug 2021 10:40:29 -0700 Subject: [PATCH 3/4] fix(pubsub): always make config check to prevent race (#4606) In the previous iteration of this PR, #4602, the assumption was made that making the config check earlier will prevent race conditions in the test, which turned out to be incorrect. In addition, although previously ruled out as a solution, this PR makes it so that a config check is always made on the first call to `Receive`. I originally thought this would result in a poor experience for those users who don't have the `roles/pubsub.viewer` permission, but we default `enableOrdering` to `true` anyway if the config call fails due to lack of permissions. Using a default of `true` only negatively impacts those who don't want ordering on their subscription, and this behavior is [already documented](https://github.com/googleapis/google-cloud-go/blob/pubsub/v1.14.0/pubsub/subscription.go#L235-L239). Fixes #3626 --- pubsub/subscription.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pubsub/subscription.go b/pubsub/subscription.go index d228ed12c33..5fa080724a3 100644 --- a/pubsub/subscription.go +++ b/pubsub/subscription.go @@ -48,7 +48,6 @@ type Subscription struct { mu sync.Mutex receiveActive bool - once sync.Once enableOrdering bool } @@ -785,6 +784,8 @@ func (s *Subscription) Receive(ctx context.Context, f func(context.Context, *Mes s.mu.Unlock() defer func() { s.mu.Lock(); s.receiveActive = false; s.mu.Unlock() }() + s.checkOrdering() + maxCount := s.ReceiveSettings.MaxOutstandingMessages if maxCount == 0 { maxCount = DefaultReceiveSettings.MaxOutstandingMessages @@ -897,9 +898,6 @@ func (s *Subscription) Receive(ctx context.Context, f func(context.Context, *Mes } for i, msg := range msgs { msg := msg - if msg.OrderingKey != "" { - s.once.Do(s.checkOrdering) - } // TODO(jba): call acquire closer to when the message is allocated. if err := fc.acquire(ctx, len(msg.Data)); err != nil { // TODO(jba): test that these "orphaned" messages are nacked immediately when ctx is done. From ee2756fb0a335d591464a770c9fa4f8fe0ba2e01 Mon Sep 17 00:00:00 2001 From: Brenna N Epp Date: Fri, 13 Aug 2021 13:41:22 -0500 Subject: [PATCH 4/4] fix(storage): preserve supplied endpoint's scheme (#4609) --- storage/storage.go | 5 ++++- storage/storage_test.go | 27 ++++++++++++++++++++++++--- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/storage/storage.go b/storage/storage.go index 82048923cd5..ed633f7208b 100644 --- a/storage/storage.go +++ b/storage/storage.go @@ -139,12 +139,15 @@ func NewClient(ctx context.Context, opts ...option.ClientOption) (*Client, error if err != nil { return nil, fmt.Errorf("storage client: %v", err) } - // Update readHost with the chosen endpoint. + // Update readHost and scheme with the chosen endpoint. u, err := url.Parse(ep) if err != nil { return nil, fmt.Errorf("supplied endpoint %q is not valid: %v", ep, err) } readHost = u.Host + if u.Scheme != "" { + scheme = u.Scheme + } return &Client{ hc: hc, diff --git a/storage/storage_test.go b/storage/storage_test.go index 55d00e2252f..8f99e60ed65 100644 --- a/storage/storage_test.go +++ b/storage/storage_test.go @@ -1252,6 +1252,7 @@ func TestAttrToFieldMapCoverage(t *testing.T) { func TestWithEndpoint(t *testing.T) { originalStorageEmulatorHost := os.Getenv("STORAGE_EMULATOR_HOST") testCases := []struct { + desc string CustomEndpoint string StorageEmulatorHost string WantRawBasePath string @@ -1259,6 +1260,7 @@ func TestWithEndpoint(t *testing.T) { WantScheme string }{ { + desc: "No endpoint or emulator host specified", CustomEndpoint: "", StorageEmulatorHost: "", WantRawBasePath: "https://storage.googleapis.com/storage/v1/", @@ -1266,6 +1268,7 @@ func TestWithEndpoint(t *testing.T) { WantScheme: "https", }, { + desc: "With specified https endpoint, no specified emulator host", CustomEndpoint: "https://fake.gcs.com:8080/storage/v1", StorageEmulatorHost: "", WantRawBasePath: "https://fake.gcs.com:8080/storage/v1", @@ -1273,6 +1276,15 @@ func TestWithEndpoint(t *testing.T) { WantScheme: "https", }, { + desc: "With specified http endpoint, no specified emulator host", + CustomEndpoint: "http://fake.gcs.com:8080/storage/v1", + StorageEmulatorHost: "", + WantRawBasePath: "http://fake.gcs.com:8080/storage/v1", + WantReadHost: "fake.gcs.com:8080", + WantScheme: "http", + }, + { + desc: "Emulator host specified, no specified endpoint", CustomEndpoint: "", StorageEmulatorHost: "http://emu.com", WantRawBasePath: "http://emu.com", @@ -1280,10 +1292,19 @@ func TestWithEndpoint(t *testing.T) { WantScheme: "http", }, { + desc: "Endpoint overrides emulator host when both are specified - https", CustomEndpoint: "https://fake.gcs.com:8080/storage/v1", StorageEmulatorHost: "http://emu.com", WantRawBasePath: "https://fake.gcs.com:8080/storage/v1", WantReadHost: "fake.gcs.com:8080", + WantScheme: "https", + }, + { + desc: "Endpoint overrides emulator host when both are specified - http", + CustomEndpoint: "http://fake.gcs.com:8080/storage/v1", + StorageEmulatorHost: "https://emu.com", + WantRawBasePath: "http://fake.gcs.com:8080/storage/v1", + WantReadHost: "fake.gcs.com:8080", WantScheme: "http", }, } @@ -1299,13 +1320,13 @@ func TestWithEndpoint(t *testing.T) { } if c.raw.BasePath != tc.WantRawBasePath { - t.Errorf("raw.BasePath not set correctly: got %v, want %v", c.raw.BasePath, tc.WantRawBasePath) + t.Errorf("%s: raw.BasePath not set correctly\n\tgot %v, want %v", tc.desc, c.raw.BasePath, tc.WantRawBasePath) } if c.readHost != tc.WantReadHost { - t.Errorf("readHost not set correctly: got %v, want %v", c.readHost, tc.WantReadHost) + t.Errorf("%s: readHost not set correctly\n\tgot %v, want %v", tc.desc, c.readHost, tc.WantReadHost) } if c.scheme != tc.WantScheme { - t.Errorf("scheme not set correctly: got %v, want %v", c.scheme, tc.WantScheme) + t.Errorf("%s: scheme not set correctly\n\tgot %v, want %v", tc.desc, c.scheme, tc.WantScheme) } } os.Setenv("STORAGE_EMULATOR_HOST", originalStorageEmulatorHost)