fix: move restart on error Actor option to Run options

[DaveHanns]

Changes related to move of restartOnError from Actor's top level to its run options.

Core: https://github.com/apify/apify-core/pull/23381
Worker: https://github.com/apify/apify-worker/pull/1515
Shared: apify/apify-shared-js#546
JS Client: apify/apify-client-js#760
Docs: apify/apify-docs#1965

[DaveHanns]

Pinging @valekjo and @gippy for reference as they are reviewing the rest of related PRs.

[fnesveda]

Shouldn't someone from tooling team who handles Python development be assigned for review too, instead of 3 platform devs?

[DaveHanns]

Shouldn't someone from tooling team who handles Python development be assigned for review too, instead of 3 platform devs?

Yep, added @janbuchar.

[janbuchar]

Shouldn't this be a fix: instead of a feat:?

[DaveHanns]

Question: @janbuchar, is this a breaking change (changing the order of the parameters) ? I'm not sure about the exact intricacies of the Python argument ordering.

[janbuchar]

Well, technically it is. But:

1. Nobody in their right mind would call this with positional (non-keyword) arguments. In fact, the entire signature of the function should probably be keyword-only, but that's breaking
2. While it is not de facto private, it probably should be

That considered, is it a problem to just move the parameter down?

[DaveHanns]

Sorry, I wasn't specific enough and didn't choose the best example. Similar changes are present throughout this and other files in public function. Please, see the last commit.

See for instance start. Same question there.

The positioning can be changed, but I would rather keep it near other similar options.
What do you think ? In other languages, it would definitely be a BC, but I'm not sure about the Python.

[janbuchar]

If the signature has this, any parameters behind the asterisk can go in any order, no BC break. What the thing means is "everything after the asterisk must be passed with a keyword (`foo(..., bar=42)`)", so the order doesn't matter.

[DaveHanns]

Nice, thanks.

[vdusek]

Yeah, if there's no asterisk in the list of args, I'd just add a new argument at the end, just to be safe.

[DaveHanns]

Ok. Only such occurrence was in get_task_representation in the task.py.
Moved to the end of args list.

[DaveHanns]

Im getting FBT001 Boolean-typed positional argument in function definition linting error.
Would adding asterix in the list help?

def get_task_representation(
    actor_id: str | None = None,
    name: str | None = None,
    task_input: dict | None = None,
    build: str | None = None,
    max_items: int | None = None,
    memory_mbytes: int | None = None,
    timeout_secs: int | None = None,
    title: str | None = None,
    actor_standby_desired_requests_per_actor_run: int | None = None,
    actor_standby_max_requests_per_actor_run: int | None = None,
    actor_standby_idle_timeout_secs: int | None = None,
    actor_standby_build: str | None = None,
    actor_standby_memory_mbytes: int | None = None,
    *,
    restart_on_error: bool | None = None,
) -> dict:

[janbuchar]

Yes, Asterix should be able to help. The lint rule exists because free-floating boolean arguments are confusing (foo(True, True, False, True)).

[DaveHanns]

Done.