Skip to content

feat: Add support for control-client stall support#776

Merged
krishnamd-jkp merged 13 commits intogoogleapis:mainfrom
gargnitingoogle:gargnitin/ctrl-client-support/v1
Apr 7, 2026
Merged

feat: Add support for control-client stall support#776
krishnamd-jkp merged 13 commits intogoogleapis:mainfrom
gargnitingoogle:gargnitin/ctrl-client-support/v1

Conversation

@gargnitingoogle
Copy link
Copy Markdown
Contributor

@gargnitingoogle gargnitingoogle commented Mar 26, 2026 •
edited
Loading

  • Adds support for the following control-client APIs in storage-testbench (only for GRPC server because control-client calls are supported only through GRPC in GCS)
    • CreateFolder
    • GetFolder
    • ListFolders
    • DeleteFolder
    • RenameFolder
  • Also adds stall support for the above APIs in storage-testbench, to test the stall-retry fix in gcsfuse for control-client APIs
  • Fixes b/493729540
  • Reuses feat(control-client-stall): adding stall feature for control client #773
  • Tests pass
  • Appropriate changes to README are included in PR

@gargnitingoogle gargnitingoogle changed the title Add support for control-client stall support feat: Add support for control-client stall support Mar 26, 2026
@gargnitingoogle gargnitingoogle force-pushed the gargnitin/ctrl-client-support/v1 branch from f4d20c0 to cd8c7b5 Compare March 26, 2026 07:19
@gargnitingoogle gargnitingoogle force-pushed the gargnitin/ctrl-client-support/v1 branch from cd8c7b5 to ce08859 Compare March 26, 2026 07:25
@gargnitingoogle
Copy link
Copy Markdown
Contributor Author

gargnitingoogle commented Mar 26, 2026

/gemini review

@gargnitingoogle gargnitingoogle marked this pull request as ready for review March 26, 2026 07:39
@gargnitingoogle gargnitingoogle requested a review from a team as a code owner March 26, 2026 07:39
@gargnitingoogle
Copy link
Copy Markdown
Contributor Author

gargnitingoogle commented Mar 26, 2026

@raj-prince FYI

@gargnitingoogle
Copy link
Copy Markdown
Contributor Author

gargnitingoogle commented Mar 26, 2026

@cpriti-os FYI

raj-prince
raj-prince reviewed Mar 31, 2026
View reviewed changes
raj-prince
raj-prince reviewed Mar 31, 2026
View reviewed changes
raj-prince
raj-prince reviewed Mar 31, 2026
View reviewed changes
raj-prince
raj-prince reviewed Mar 31, 2026
View reviewed changes
raj-prince
raj-prince reviewed Mar 31, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@raj-prince raj-prince left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM, added few minor comments.

Also, good to test overall with this change, by creating a custom docker image.

@gargnitingoogle
add Prince review comment
69876ea
@gargnitingoogle
Changed control-client unit tests to table-based
abcb078 
as per review comment given by Prince.
Also added a new unit test to test a bit
more complex scenario of first stalling 2 times,
and then trying out without stall to see that
it works.
@gargnitingoogle gargnitingoogle force-pushed the gargnitin/ctrl-client-support/v1 branch from 11d6f63 to abcb078 Compare April 1, 2026 06:40
raj-prince
raj-prince reviewed Apr 1, 2026
View reviewed changes
@gargnitingoogle gargnitingoogle mentioned this pull request Apr 2, 2026
Open
cpriti-os
cpriti-os reviewed Apr 3, 2026
View reviewed changes
cpriti-os
cpriti-os reviewed Apr 3, 2026
View reviewed changes
cpriti-os
cpriti-os reviewed Apr 3, 2026
View reviewed changes
@gargnitingoogle gargnitingoogle force-pushed the gargnitin/ctrl-client-support/v1 branch from b3d7be7 to defa341 Compare April 3, 2026 06:08
@gargnitingoogle gargnitingoogle force-pushed the gargnitin/ctrl-client-support/v1 branch from defa341 to ddf99df Compare April 3, 2026 06:11
krishnamd-jkp
krishnamd-jkp reviewed Apr 6, 2026
View reviewed changes
@gargnitingoogle gargnitingoogle force-pushed the gargnitin/ctrl-client-support/v1 branch from 31fb452 to b0fe216 Compare April 6, 2026 09:08
@gargnitingoogle gargnitingoogle force-pushed the gargnitin/ctrl-client-support/v1 branch from b0fe216 to 1564119 Compare April 6, 2026 09:12
@gargnitingoogle
Copy link
Copy Markdown
Contributor Author

gargnitingoogle commented Apr 7, 2026 •
edited
Loading

@cpriti-os, @raj-prince and @krishnamd-jkp have already reviewed and approved. PTAL.

There is one more thing that I want to point out.
I wrote and was running the tests in gcsfuse for testing these control-client functions, and found that more changes are required for RenameFolder in storage-testbench, as it's a long-running operation which has a different path in the proto files, and also has a different signature from what gemini imagined it to be, so I have added some more changes to make that work, but have not pushed them to this PR yet. I am thinking of not adding them here, to avoid increasing this one in size. What do you think: should I add those changes here too, or open a new PR for them ?

@cpriti-os
Copy link
Copy Markdown
Contributor

cpriti-os commented Apr 7, 2026

@cpriti-os, @raj-prince and @krishnamd-jkp have already reviewed and approved. PTAL.

There is one more thing that I want to point out. I wrote and was running the tests in gcsfuse for testing these control-client functions, and found that more changes are required for RenameFolder in storage-testbench, as it's a long-running operation which has a different path in the proto files, and also has a different signature from what gemini imagined it to be, so I have added some more changes to make that work, but have not pushed them to this PR yet. I am thinking of not adding them here, to avoid increasing this one in size. What do you think: should I add those changes here too, or open a new PR for them ?

New PR sounds good to me to keep changes small and isolated.

@gargnitingoogle gargnitingoogle mentioned this pull request Apr 7, 2026
Open
2 tasks
@gargnitingoogle
Copy link
Copy Markdown
Contributor Author

gargnitingoogle commented Apr 7, 2026

@cpriti-os, @raj-prince and @krishnamd-jkp have already reviewed and approved. PTAL.
There is one more thing that I want to point out. I wrote and was running the tests in gcsfuse for testing these control-client functions, and found that more changes are required for RenameFolder in storage-testbench, as it's a long-running operation which has a different path in the proto files, and also has a different signature from what gemini imagined it to be, so I have added some more changes to make that work, but have not pushed them to this PR yet. I am thinking of not adding them here, to avoid increasing this one in size. What do you think: should I add those changes here too, or open a new PR for them ?

New PR sounds good to me to keep changes small and isolated.

@cpriti-os Then can you approve this one? I am not able to merge this one without your approval, I think because you have reviewed it once already.

The additional changes for RenameFolder are in #786 FYI.

@krishnamd-jkp krishnamd-jkp merged commit e2a2cf3 into googleapis:main Apr 7, 2026
11 checks passed
@gargnitingoogle gargnitingoogle deleted the gargnitin/ctrl-client-support/v1 branch April 7, 2026 07:01
cpriti-os
cpriti-os reviewed Apr 7, 2026
View reviewed changes
cpriti-os
cpriti-os reviewed Apr 7, 2026
View reviewed changes
cpriti-os
cpriti-os reviewed Apr 7, 2026
View reviewed changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cpriti-os cpriti-os cpriti-os left review comments

@krishnamd-jkp krishnamd-jkp krishnamd-jkp approved these changes

+1 more reviewer

@raj-prince raj-prince raj-prince approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@gargnitingoogle @cpriti-os @raj-prince @krishnamd-jkp