Skip to content

Conversation

@ddelgrosso1
Copy link
Contributor

@ddelgrosso1 ddelgrosso1 commented Sep 30, 2024

Fixes: #14396


This change is Reviewable

@product-auto-label product-auto-label bot added the api: storage Issues related to the Cloud Storage API. label Sep 30, 2024
@ddelgrosso1 ddelgrosso1 force-pushed the remove-grpc-notification branch from 468e0b6 to da14840 Compare September 30, 2024 14:59
@ddelgrosso1 ddelgrosso1 changed the title fix(storage): make notification ops return unimplemented in gRPC fix(storage): make notification and hmac ops return unimplemented in gRPC Sep 30, 2024
@ddelgrosso1 ddelgrosso1 changed the title fix(storage): make notification and hmac ops return unimplemented in gRPC fix(storage): make notification, hmac, service account ops return unimplemented in gRPC Sep 30, 2024
@ddelgrosso1 ddelgrosso1 marked this pull request as ready for review September 30, 2024 16:53
@ddelgrosso1 ddelgrosso1 requested review from a team as code owners September 30, 2024 16:53
Copy link
Member

@scotthart scotthart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 24 files reviewed, 1 unresolved discussion (waiting on @ddelgrosso1)


google/cloud/storage/internal/grpc/stub.cc line 973 at r1 (raw file):

    rest_internal::RestContext&, Options const&,
    storage::internal::GetProjectServiceAccountRequest const&) {
  return Status(StatusCode::kUnimplemented, "");

If these rpcs are available in storagecontrol, would it be useful for the for the error message to indicate such?

@codecov
Copy link

codecov bot commented Sep 30, 2024

Codecov Report

Attention: Patch coverage is 52.38095% with 10 lines in your changes missing coverage. Please review.

Project coverage is 93.42%. Comparing base (060a8f9) to head (44e170b).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
google/cloud/storage/internal/grpc/stub.cc 0.00% 10 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #14742      +/-   ##
==========================================
- Coverage   93.45%   93.42%   -0.03%     
==========================================
  Files        2323     2314       -9     
  Lines      208152   207591     -561     
==========================================
- Hits       194529   193945     -584     
- Misses      13623    13646      +23     
Flag Coverage Δ
93.42% <52.38%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor Author

@ddelgrosso1 ddelgrosso1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 24 files reviewed, 1 unresolved discussion (waiting on @scotthart)


google/cloud/storage/internal/grpc/stub.cc line 973 at r1 (raw file):

Previously, scotthart (Scott Hart) wrote…

If these rpcs are available in storagecontrol, would it be useful for the for the error message to indicate such?

They aren't yet available in storage control but I will add myself a followup issue to update these messages once they become available.

Copy link
Member

@scotthart scotthart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 24 of 24 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ddelgrosso1)

@ddelgrosso1 ddelgrosso1 merged commit 762c5ac into googleapis:main Sep 30, 2024
@ddelgrosso1 ddelgrosso1 deleted the remove-grpc-notification branch September 30, 2024 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: storage Issues related to the Cloud Storage API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Figure out what to do with the Notification RPCs in the GCS+gRPC plugin

2 participants