refactor!: Mark secondary arguments as keyword-only#917
Conversation
Secondary parameters (default_value, charged_event_name, count, key, kvs_name) on Actor.get_value, Actor.push_data, Actor.charge, Actor.use_state, and ChargingManager.charge are now keyword-only. Primary subjects such as key, data, and event_name stay positional. Improves call-site readability and prevents argument-order mistakes when extending signatures. Mirrors apify-client#766. BREAKING CHANGE: callers passing the affected arguments positionally must switch to keyword form. See docs/04_upgrading/upgrading_to_v4.md for details.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #917 +/- ##
==========================================
+ Coverage 87.00% 87.04% +0.03%
==========================================
Files 48 48
Lines 2956 2956
==========================================
+ Hits 2572 2573 +1
+ Misses 384 383 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| ## Keyword-only arguments | ||
|
|
||
| Secondary parameters on these methods can no longer be passed positionally. |
There was a problem hiding this comment.
| Secondary parameters on these methods can no longer be passed positionally. | |
| Secondary parameters in these signatures can no longer be passed positionally: | |
| - `Actor` — `get_value`, `push_data`, `charge`, `use_state`. | |
| - `ChargingManager` — `charge`. |
It's a little awkward that we start with these methods but don't list them right away. How about something like this?
| Secondary parameters on these methods can no longer be passed positionally. | ||
|
|
||
| ```python | ||
| # Before |
There was a problem hiding this comment.
| # Before | |
| # Before (v3) |
| await Actor.push_data(data, 'my-event') | ||
| await Actor.charge('my-event', 5) | ||
|
|
||
| # After |
There was a problem hiding this comment.
| # After | |
| # After (v4) |
|
|
||
| @_ensure_context | ||
| async def push_data(self, data: dict | list[dict], charged_event_name: str | None = None) -> ChargeResult: | ||
| async def push_data(self, data: dict | list[dict], *, charged_event_name: str | None = None) -> ChargeResult: |
There was a problem hiding this comment.
Not so sure about this — charged_event_name is pretty long. Since this is a pretty important part of the API, can you please get an OK from someone in the Publishers team perhaps? Or whoever is responsible for PPE...
Summary
Closes #881.
Reshapes function/method signatures across the SDK public API so that secondary parameters must be passed as keyword arguments. Primary "subject" arguments (e.g.
key,data,event_name) stay positional. Mirrors what was done in the client: apify/apify-client-python#766.Affected APIs
Actor:get_value(key, *, default_value=None)push_data(data, *, charged_event_name=None)charge(event_name, *, count=1)use_state(default_value=None, *, key=None, kvs_name=None)ChargingManager/ChargingManagerImplementation(returned byActor.get_charging_manager()):charge(event_name, *, count=1)Crawlee-overriding methods (e.g.
ProxyConfiguration.new_proxy_info, the storage clients) were intentionally left untouched — Crawlee's base signatures are positional, so making the overrides keyword-only would diverge from the base class.Why
Keyword-only parameters at API boundaries make call sites self-documenting and prevent breakage when new options are added between existing arguments.
BREAKING CHANGE for v4.0 — see
docs/04_upgrading/upgrading_to_v4.mdfor the migration guide.