Skip to content

Conversation

mario-vimal
Copy link
Owner

No description provided.

@@ -0,0 +1,130 @@

//
// Copyright 2020 gRPC authors.

Choose a reason for hiding this comment

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

super nit: 2023

#include "src/core/lib/json/json_reader.h"
#include "src/core/lib/json/json_writer.h"
#include "src/core/lib/security/credentials/credentials.h"
#include "src/core/lib/uri/uri_parser.h"

Choose a reason for hiding this comment

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

nit: Do we need all these includes? Wondering about the uri_parser in particular

Copy link
Owner Author

Choose a reason for hiding this comment

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

We need not worry about includes as the "include what you use" iwyu tool will show us the unused and missed includes during build.
I've modified the includes for now by running iwyu locally, which isn't that reliable.

Choose a reason for hiding this comment

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

oh great, no worries then

if (executable_it != executable_json.object().end()) {
if (!absl::SimpleAtoi(executable_it->second.string(),
&executable_timeout_ms_)) {
*error = GRPC_ERROR_CREATE("timeout_millis field must be a number");

Choose a reason for hiding this comment

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

nit: add period at the end of the error message

mario-vimal pushed a commit that referenced this pull request Jul 13, 2023
This adds pre-built library for aarch64 linux, will help improve the
install speed and avoid building environment issues at customer side.

@apolcyn @jtattermusch Can you help build and push the new rake compiler
image?
Will update the tag and hash after the image is available

Manually tested locally:
```
uname -a
Linux u20 5.15.49-linuxkit #1 SMP PREEMPT Tue Sep 13 07:51:32 UTC 2022 aarch64 aarch64 aarch64 GNU/Linux
```
```
time gem install /work/ruby/grpc/pkg/grpc-1.56.0.dev-aarch64-linux.gem
Successfully installed grpc-1.56.0.dev-aarch64-linux
Parsing documentation for grpc-1.56.0.dev-aarch64-linux
Installing ri documentation for grpc-1.56.0.dev-aarch64-linux
Done installing documentation for grpc after 0 seconds
1 gem installed

real	0m22.794s
user	0m17.268s
sys	0m5.156s
```
```
ruby greeter_server.rb &
[1] 319
ruby greeter_client.rb
"Greeting: Hello world"
```

Fixes:
grpc#31855
grpc#29489
mario-vimal pushed a commit that referenced this pull request Jul 13, 2023
This adds pre-built library for aarch64 linux, will help improve the
install speed and avoid building environment issues at customer side.

@apolcyn @jtattermusch Can you help build and push the new rake compiler
image?
Will update the tag and hash after the image is available

Manually tested locally:
```
uname -a
Linux u20 5.15.49-linuxkit #1 SMP PREEMPT Tue Sep 13 07:51:32 UTC 2022 aarch64 aarch64 aarch64 GNU/Linux
```
```
time gem install /work/ruby/grpc/pkg/grpc-1.56.0.dev-aarch64-linux.gem
Successfully installed grpc-1.56.0.dev-aarch64-linux
Parsing documentation for grpc-1.56.0.dev-aarch64-linux
Installing ri documentation for grpc-1.56.0.dev-aarch64-linux
Done installing documentation for grpc after 0 seconds
1 gem installed

real	0m22.794s
user	0m17.268s
sys	0m5.156s
```
```
ruby greeter_server.rb &
[1] 319
ruby greeter_client.rb
"Greeting: Hello world"
```

Fixes:
grpc#31855
grpc#29489
@mario-vimal mario-vimal requested a review from aeitzman July 14, 2023 18:46

TEST(
CredentialsTest,
TestPluggableAuthExternalAccountCredentialsCreateFailureExecutableTimeoutLessThanMin) {

Choose a reason for hiding this comment

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

nit: less than 5 seconds?

@mario-vimal mario-vimal requested a review from aeitzman July 14, 2023 20:38
@mario-vimal mario-vimal merged commit 29492f8 into pluggable_auth Jul 18, 2023
mario-vimal added a commit that referenced this pull request Jul 27, 2023
* Add pluggable auth support for external account creds

* Add tests for pluggable auth - ADC

* Update test name - credentials_test.cc
mario-vimal added a commit that referenced this pull request Jul 27, 2023
* Add pluggable auth support for external account creds

* Add tests for pluggable auth - ADC

* Update test name - credentials_test.cc
mario-vimal added a commit that referenced this pull request Jul 27, 2023
* Add pluggable auth support for external account creds

* Add tests for pluggable auth - ADC

* Update test name - credentials_test.cc
mario-vimal added a commit that referenced this pull request Jul 27, 2023
* Add pluggable auth support for external account creds

* Add tests for pluggable auth - ADC

* Update test name - credentials_test.cc
mario-vimal added a commit that referenced this pull request Jul 27, 2023
* Add pluggable auth support for external account creds

* Add tests for pluggable auth - ADC

* Update test name - credentials_test.cc
mario-vimal added a commit that referenced this pull request Aug 2, 2023
* Add pluggable auth support for external account creds

* Add tests for pluggable auth - ADC

* Update test name - credentials_test.cc
mario-vimal added a commit that referenced this pull request Aug 2, 2023
* Add pluggable auth support for external account creds

* Add tests for pluggable auth - ADC

* Update test name - credentials_test.cc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants