Skip to content

fix(gapic-common): ignore inherited Service constants in normalize_service - #71

Open
linyiru wants to merge 1 commit into
googleapis:mainfrom
linyiru:fix/normalize-service-inherit
Open

linyiru wants to merge 1 commit into
googleapis:mainfrom
linyiru:fix/normalize-service-inherit

Conversation

@linyiru

@linyiru linyiru commented Sep 11, 2026

Copy link
Copy Markdown

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea.
  • Follow the instructions in CONTRIBUTING. Most importantly, ensure the tests and linter pass by running toys ci in the gem subdirectory.
  • Update code documentation if necessary.

closes: #70

Summary

Gapic::LoggingConcerns.normalize_service looks up the Service constant with const_defined?'s default inherit: true. Generated REST stubs have no sibling Service, so the lookup walks up to Object. If the application defines a top-level ::Service, it is treated as a gRPC Service and service_name is called on it, raising NoMethodError during client construction.

gRPC transports are unaffected because they do define a local Service.

Change

Look up Service and Rest with inherit: false so only constants defined on the enclosing module are considered.

Tests

On starship (Ruby 3.4.10):

  • toys test: 316 runs, 948 assertions, 0 failures, 0 errors
  • bundle exec rubocop lib: 42 files, no offenses
  • bundle exec ruby -Ilib:test test/gapic/logging_concerns_test.rb: 10 runs, 12 assertions, 0 failures

…rvice

REST stubs have no sibling Service constant, so const_defined? with
the default inherit: true walks up to Object. An application-level
::Service is then treated as a gRPC Service and service_name is
called on it, raising NoMethodError during client construction.

Look up Service and Rest with inherit: false so only locally defined
constants are considered.

Fixes googleapis#70
@linyiru
linyiru requested a review from a team as a code owner September 11, 2026 03:25
@google-cla

google-cla Bot commented Sep 11, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@linyiru

linyiru commented Sep 15, 2026

Copy link
Copy Markdown
Author

CLA is signed and green now.

The CI workflow run is sitting in action_required — as a first-time contributor from a fork, it needs a maintainer to click "Approve and run workflows" before anything starts. Could someone kick it off, and take a look when you get a chance?

Ran the suite locally in the meantime (Ruby 3.3.10, macOS):

  • gapic-common tests: 316 runs, 948 assertions, 0 failures, 0 errors, 1 skip
  • rubocop: 44 files inspected, no offenses

Happy to adjust anything.

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.

normalize_service resolves "Service" with inherit: true, breaking REST transports in apps with a top-level ::Service

1 participant