Skip to content

fix: restore subclass dispatch for _apply_filter in ServiceMetadataProvider#3067

Open
Youssef-SH wants to merge 1 commit into
Netflix:masterfrom
Youssef-SH:fix/provider-dispatch-apply-filter
Open

fix: restore subclass dispatch for _apply_filter in ServiceMetadataProvider#3067
Youssef-SH wants to merge 1 commit into
Netflix:masterfrom
Youssef-SH:fix/provider-dispatch-apply-filter

Conversation

@Youssef-SH
Copy link
Copy Markdown

Restore subclass dispatch for _apply_filter in ServiceMetadataProvider.

ServiceMetadataProvider._get_object_internal calls
MetadataProvider._apply_filter(...) directly, which bypasses subclass overrides.

Switch to cls._apply_filter(...) to preserve current behavior for built-in
providers while allowing subclasses to override filtering as expected.

Adds a regression test covering both single-object and collection paths.

PR Type

  • Bug fix
  • New feature
  • Core Runtime change (higher bar -- see CONTRIBUTING.md)
  • Docs / tooling
  • Refactoring

Summary

Restore correct subclass dispatch for _apply_filter in ServiceMetadataProvider.

Issue

Fixes #

Reproduction

Covered by the added unit test in test/unit/test_service_metadata_provider.py.

Root Cause

ServiceMetadataProvider._get_object_internal calls _apply_filter through the
base class (MetadataProvider._apply_filter(...)) instead of cls, so subclass
overrides are ignored.

Why This Fix Is Correct

Using cls._apply_filter(...) restores normal method dispatch without changing
behavior for built-in providers, since none override _apply_filter.

Failure Modes Considered

  1. No behavior change for built-in providers.
  2. Subclasses overriding _apply_filter now behave as expected.

Tests

  • Unit tests added/updated
  • Reproduction script provided (required for Core Runtime)
  • CI passes
  • If tests are impractical: explain why below and provide manual evidence above

Non-Goals

No changes to filtering logic or provider behavior beyond dispatch.

AI Tool Usage

  • No AI tools were used in this contribution
  • AI tools were used (describe below)

Used for review and validation. All code and tests were verified manually.

ServiceMetadataProvider._get_object_internal calls
MetadataProvider._apply_filter(...) directly, which bypasses subclass overrides.

Switch to cls._apply_filter(...) to restore correct dispatch while preserving
existing behavior.

Adds a regression test to ensure subclass overrides are used.
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 30, 2026

Greptile Summary

This PR fixes a long-standing subclass dispatch bug in ServiceMetadataProvider._get_object_internal: both call sites were invoking MetadataProvider._apply_filter(...) directly, bypassing any subclass override. Switching to cls._apply_filter(...) is the standard Python idiom for honouring MRO in classmethods and is a zero-risk change for the built-in providers (none of them override _apply_filter). The fix is accompanied by a focused regression test that exercises both the single-object and collection paths via a fake subclass.

Key points:

  • The two-line change in service.py is minimal and precisely targeted.
  • _apply_filter is defined as a @staticmethod on the base class; calling it as cls._apply_filter(...) still dispatches correctly through the MRO, so the approach is sound.
  • The test covers exactly the two call sites that were broken, and assertions are tight enough to catch a regression.
  • One minor note: the test's _apply_filter override ignores the filters argument entirely (it always filters by the keep field). This is intentional for the test's purposes but means the test does not verify that filters is forwarded correctly to the override — a marginal gap that is unlikely to matter in practice.

Confidence Score: 5/5

  • Safe to merge — minimal two-line fix with no behavior change for existing providers and a focused regression test.
  • All findings are P2 or lower. The core fix is correct, well-scoped, and zero-risk for built-in providers. The test adequately covers both affected code paths. No P0/P1 issues were identified.
  • No files require special attention.

Important Files Changed

Filename Overview
metaflow/plugins/metadata_providers/service.py Two-line fix replacing MetadataProvider._apply_filter(...) with cls._apply_filter(...) in both the single-object and collection paths, restoring proper subclass dispatch.
test/unit/test_service_metadata_provider.py New regression test with a fake subclass overriding both _request and _apply_filter, covering both the single-object (sub_type == "self") and collection paths.

Reviews (1): Last reviewed commit: "fix: use subclass overrides of _apply_fi..." | Re-trigger Greptile

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.

1 participant