Repository navigation
tech-debt: make option-heavy public signatures keyword-only #1372
Description
Component
Python SDK
Need
The SDK's most-used client methods accept a long tail of optional arguments positionally. filters has 19 positional-capable parameters, all 18 and get 17 (each roughly doubled across the async and sync clients), and 25 public callables sit at 8 or more.
Only the leading argument is genuinely positional in practice - kind for the client query methods - and everything after it is an option. Because those options are positionally bindable, any change to their order or any insertion between them is a silent breaking change: existing callers keep working but rebind a value to a different parameter, with no error and no type-checker signal.
This is not hypothetical. During review of #1120 a new merge option was added to get, all and filters between populate_store and fragment, and a review bot correctly pointed out that a caller passing fragment or anything after it positionally would now bind it to merge instead. We judged the risk acceptable for that PR on the grounds that nobody realistically passes eleven positional arguments, and left the parameter in its logical position next to populate_store. That reasoning holds for one change, but it does not remove the hazard - it just leaves it in place for the next person to add an option.
Enforcing keyword-only arguments after the leading required one would make this class of mistake impossible and let options be grouped by meaning rather than by arrival order.
Use case
Two audiences benefit:
- SDK maintainers gain the freedom to add, reorder or group options without auditing for positional callers, and lose an entire category of silent breakage in review.
- SDK users get an immediate
TypeErrorinstead of a value quietly landing in the wrong parameter. Call sites also become self-describing, which matters most in the network-automation scripts and generators this SDK is written for, where a misbound boolean can change what is written to the source of truth.
Additional information
The change is breaking and needs a major release. The break is shallow - it only affects callers already passing options positionally, which is the same population the merge discussion was about - but it is a real signature change and should be called out in the changelog and upgrade notes.
Worth deciding as part of the work:
- Where the keyword-only boundary sits for each callable (for the client query methods, most likely immediately after
kind, withraise_when_missinga judgement call). - Whether to cover
infrahubctlcommand functions and internal helpers, or restrict the change to the documented public API. - Whether a deprecation period is feasible ahead of the major, for example warning on positional use before enforcing it.
The 25 affected callables can be regenerated by walking the AST of infrahub_sdk/ for public functions whose positional-capable parameter count crosses a threshold; the count above used 8.
Raised during review of #1120.
Assisted-by: opsmill-dev-creating-issues 0.2.0
Activity
fix(sdk): close store-merge gaps found in review