Skip to content

Fix ruby inherited methods - #15

Merged
tgallice merged 2 commits into
masterfrom
tg/processor-inherited-methods
Sep 16, 2026
Merged

tgallice merged 2 commits into
masterfrom
tg/processor-inherited-methods

Conversation

@tgallice

Copy link
Copy Markdown
Member

Problem

Generated processors now dispatch only through the METHODS table (the process_<name> methods are gone). When a service extends another one, the child processor inherits from the parent processor. self.class::METHODS only returns the child's constant, so the parent's methods are never registered.

As a result, every inherited method fails with Unknown function <name>. For example, add_influencers on analytics.manager.Manager, which extends base.EntityManager.

Fix

Thrift::Processor#initialize now walks the class ancestors and merges every METHODS table it finds. A method defined in the child still takes precedence over the parent's.

Tests

  • Added a spec with a processor extending another one. It calls both an inherited method and one of its own.
  • Checked against the real Analytics::Thrift::Manager::Processor (analytics-thrift-v2 0.0.19, base-thrift-v2 0.1.176):
    • before: Unknown function add_influencers
    • after: normal reply
  • Did you create an Apache Jira ticket? (not required for trivial changes)
  • If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
  • Did you squash your changes to a single commit? (not required, but preferred)
  • Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change"?
  • If your change does not involve any code, add [skip ci] at the end of your pull request to free up build resources.

Comment thread lib/rb/lib/thrift/processor.rb Outdated
@tgallice
tgallice force-pushed the tg/processor-inherited-methods branch from fdaed3d to 689e077 Compare September 16, 2026 14:45
@tgallice
tgallice merged commit 6508600 into master Sep 16, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants