Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

interop: Custom creds for stress test client #6809

Merged
merged 3 commits into from
Nov 27, 2023

Conversation

temawi
Copy link
Contributor

@temawi temawi commented Nov 17, 2023

Allows custom, Google specific credentials to be specified from the command line.

Also adds a final printout to stdout with the total amount of calls made. Stress tests can use that to determine the average QPS of a run.

RELEASE NOTES: none

Allows custom, Google specific credentials to be specified from the
command line.

Also adds a final printout to stdout with the total amount of calls
made. Stress tests can use that to determine the average QPS of a run.
@temawi
Copy link
Contributor Author

temawi commented Nov 17, 2023

@dfawley Could you please help get this merged?

Copy link

codecov bot commented Nov 17, 2023

Codecov Report

Merging #6809 (853dcf1) into master (7935c4f) will increase coverage by 0.08%.
Report is 2 commits behind head on master.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6809      +/-   ##
==========================================
+ Coverage   83.40%   83.48%   +0.08%     
==========================================
  Files         285      285              
  Lines       30879    30879              
==========================================
+ Hits        25754    25780      +26     
+ Misses       4053     4028      -25     
+ Partials     1072     1071       -1     

see 22 files with indirect coverage changes

@dfawley
Copy link
Member

dfawley commented Nov 27, 2023

Please make sure vet is passing. You can run vet.sh locally before pushing your changes, too. You probably want to configure your editor to automatically run gofmt and optionally goimport when saving files.

@dfawley dfawley assigned temawi and unassigned dfawley Nov 27, 2023
interop/stress/client/main.go Outdated Show resolved Hide resolved
Comment on lines +292 to +299
if *customCredentialsType != "" {
if *customCredentialsType == googleDefaultCredsName {
opts = append(opts, grpc.WithCredentialsBundle(google.NewDefaultCredentials()))
} else if *customCredentialsType == computeEngineCredsName {
opts = append(opts, grpc.WithCredentialsBundle(google.NewComputeEngineCredentials()))
} else {
logger.Fatalf("Unknown custom credentials: %v", *customCredentialsType)
}
Copy link
Member

Choose a reason for hiding this comment

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

Consider:

switch *customCredentialsType {
	case googleDefaultCredsName:
	case computeEngineCredsName:
	case "":
		if useTLS {
		}
	default:
		// unknown custom creds error
}

Also if we can change the spec here, consider custom_creds_type=="tls" means to use tls instead of having a separate flag for it. Having two flags for one setting is problematic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's how the flags work in the interop test client across languages as well as the java stress test one. Makes sense to revisit, but I think that would be a broad change outside the scope of this PR.

@dfawley dfawley modified the milestone: 1.61 Release Nov 27, 2023
@dfawley dfawley merged commit bc16b5f into grpc:master Nov 27, 2023
13 checks passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 26, 2024
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants