Skip to content

Commit

Permalink
Change flag name and invert the bool
Browse files Browse the repository at this point in the history
  • Loading branch information
gnossen committed Aug 19, 2024
1 parent acaecfd commit 8d7e329
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 17 deletions.
16 changes: 10 additions & 6 deletions src/core/lib/config/config_vars.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,10 @@ ABSL_FLAG(absl::optional<bool>, grpc_not_use_system_ssl_roots, {},
"Disable loading system root certificates.");
ABSL_FLAG(absl::optional<std::string>, grpc_ssl_cipher_suites, {},
"A colon separated list of cipher suites to use with OpenSSL");
ABSL_FLAG(absl::optional<bool>, grpc_cpp_enable_reflection, {},
"Whether to automatically add a reflection handler.");
ABSL_FLAG(absl::optional<bool>, grpc_cpp_experimental_disable_reflection, {},
"EXPERIMENTAL. Only respected when there is a dependency on "
":grpc++_reflection. If true, no reflection server will be "
"automatically added.");

namespace grpc_core {

Expand All @@ -91,9 +93,10 @@ ConfigVars::ConfigVars(const Overrides& overrides)
not_use_system_ssl_roots_(LoadConfig(
FLAGS_grpc_not_use_system_ssl_roots, "GRPC_NOT_USE_SYSTEM_SSL_ROOTS",
overrides.not_use_system_ssl_roots, false)),
cpp_enable_reflection_(LoadConfig(FLAGS_grpc_cpp_enable_reflection,
"GRPC_CPP_ENABLE_REFLECTION",
overrides.cpp_enable_reflection, true)),
cpp_experimental_disable_reflection_(
LoadConfig(FLAGS_grpc_cpp_experimental_disable_reflection,
"GRPC_CPP_EXPERIMENTAL_DISABLE_REFLECTION",
overrides.cpp_experimental_disable_reflection, false)),
dns_resolver_(LoadConfig(FLAGS_grpc_dns_resolver, "GRPC_DNS_RESOLVER",
overrides.dns_resolver, "")),
verbosity_(LoadConfig(FLAGS_grpc_verbosity, "GRPC_VERBOSITY",
Expand Down Expand Up @@ -142,7 +145,8 @@ std::string ConfigVars::ToString() const {
absl::CEscape(DefaultSslRootsFilePath()), "\"",
", not_use_system_ssl_roots: ", NotUseSystemSslRoots() ? "true" : "false",
", ssl_cipher_suites: ", "\"", absl::CEscape(SslCipherSuites()), "\"",
", cpp_enable_reflection: ", CppEnableReflection() ? "true" : "false");
", cpp_experimental_disable_reflection: ",
CppExperimentalDisableReflection() ? "true" : "false");
}

} // namespace grpc_core
12 changes: 8 additions & 4 deletions src/core/lib/config/config_vars.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class GPR_DLL ConfigVars {
absl::optional<bool> enable_fork_support;
absl::optional<bool> abort_on_leaks;
absl::optional<bool> not_use_system_ssl_roots;
absl::optional<bool> cpp_enable_reflection;
absl::optional<bool> cpp_experimental_disable_reflection;
absl::optional<std::string> dns_resolver;
absl::optional<std::string> verbosity;
absl::optional<std::string> poll_strategy;
Expand Down Expand Up @@ -98,8 +98,12 @@ class GPR_DLL ConfigVars {
bool NotUseSystemSslRoots() const { return not_use_system_ssl_roots_; }
// A colon separated list of cipher suites to use with OpenSSL
absl::string_view SslCipherSuites() const { return ssl_cipher_suites_; }
// Whether to automatically add a reflection handler.
bool CppEnableReflection() const { return cpp_enable_reflection_; }
// EXPERIMENTAL. Only respected when there is a dependency on
// :grpc++_reflection. If true, no reflection server will be automatically
// added.
bool CppExperimentalDisableReflection() const {
return cpp_experimental_disable_reflection_;
}

private:
explicit ConfigVars(const Overrides& overrides);
Expand All @@ -109,7 +113,7 @@ class GPR_DLL ConfigVars {
bool enable_fork_support_;
bool abort_on_leaks_;
bool not_use_system_ssl_roots_;
bool cpp_enable_reflection_;
bool cpp_experimental_disable_reflection_;
std::string dns_resolver_;
std::string verbosity_;
std::string poll_strategy_;
Expand Down
6 changes: 3 additions & 3 deletions src/core/lib/config/config_vars.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@
ECDHE-ECDSA-AES256-GCM-SHA384:\
ECDHE-RSA-AES128-GCM-SHA256:\
ECDHE-RSA-AES256-GCM-SHA384"
- name: cpp_enable_reflection
- name: cpp_experimental_disable_reflection
type: bool
description: Whether to automatically add a reflection handler.
default: true
description: "EXPERIMENTAL. Only respected when there is a dependency on :grpc++_reflection. If true, no reflection server will be automatically added."
default: false
6 changes: 3 additions & 3 deletions src/cpp/ext/proto_server_reflection_plugin.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ void ProtoServerReflectionPlugin::InitServer(grpc::ServerInitializer* si) {
// We cannot simply keep the plugin from being unregistered because this must
// happen at static initialization time, whereas flag configuration that
// controls this is not received until later.
if (grpc_core::ConfigVars::Get().CppEnableReflection()) {
if (!grpc_core::ConfigVars::Get().CppExperimentalDisableReflection()) {
si->RegisterService(reflection_service_v1_);
si->RegisterService(reflection_service_v1alpha_);
}
Expand All @@ -59,7 +59,7 @@ void ProtoServerReflectionPlugin::ChangeArguments(const std::string& /*name*/,
void* /*value*/) {}

bool ProtoServerReflectionPlugin::has_sync_methods() const {
if (grpc_core::ConfigVars::Get().CppEnableReflection()) {
if (!grpc_core::ConfigVars::Get().CppExperimentalDisableReflection()) {
return (reflection_service_v1_ &&
reflection_service_v1_->has_synchronous_methods()) ||
(reflection_service_v1alpha_ &&
Expand All @@ -69,7 +69,7 @@ bool ProtoServerReflectionPlugin::has_sync_methods() const {
}

bool ProtoServerReflectionPlugin::has_async_methods() const {
if (grpc_core::ConfigVars::Get().CppEnableReflection()) {
if (!grpc_core::ConfigVars::Get().CppExperimentalDisableReflection()) {
return (reflection_service_v1_ &&
reflection_service_v1_->has_async_methods()) ||
(reflection_service_v1alpha_ &&
Expand Down
4 changes: 3 additions & 1 deletion tools/codegen/core/gen_config_vars.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@
print("config has no name: %r" % attr)
error = True
continue
if "experiment" in attr["name"] and attr["name"] != "experiments":
if (
"experiment" in attr["name"] and "experimental" not in attr["name"]
) and attr["name"] != "experiments":
print("use experiment system for experiments")
error = True
if "description" not in attr:
Expand Down

0 comments on commit 8d7e329

Please sign in to comment.