Skip to content

Make scan_plan_helper internal (#3541)#3553

Open
tanmayrauth wants to merge 1 commit into
apache:mainfrom
tanmayrauth:make-scan-plan-helper-internal-3541
Open

Make scan_plan_helper internal (#3541)#3553
tanmayrauth wants to merge 1 commit into
apache:mainfrom
tanmayrauth:make-scan-plan-helper-internal-3541

Conversation

@tanmayrauth

@tanmayrauth tanmayrauth commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

scan_plan_helper is an internal helper for local scan planning, not a public extension point. Rename it to _scan_plan_helper to reflect its intended visibility and update its only caller in inspect.py.

The method was introduced recently in d99936a and has not been part of any released public API, so no deprecation shim is required.
Fixes: #3541

Rationale for this change

Are these changes tested?

Are there any user-facing changes?

scan_plan_helper is an internal helper for local scan planning, not a
public extension point. Rename it to _scan_plan_helper to reflect its
intended visibility and update its only caller in inspect.py.

The method was introduced recently in d99936a and has not been part of
any released public API, so no deprecation shim is required.
@ebyhr

ebyhr commented Jun 23, 2026

Copy link
Copy Markdown
Member

Fixes: #3451

I think the right issue number is #3541

@rambleraptor rambleraptor left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Easy peasy. Thanks for the PR!

return self._manifest_planner.partition_filters

def scan_plan_helper(self) -> Iterator[list[ManifestEntry]]:
def _scan_plan_helper(self) -> Iterator[list[ManifestEntry]]:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: I feel _helper suffix is weird. The name should describe what it does, not that it's a helper. We could rename it to _plan_manifest_entries or something.

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.

Make scan_plan_helper internal

4 participants