Skip to content

Conversation

@matyascimbulka
Copy link

@matyascimbulka matyascimbulka commented Dec 15, 2025

Implements #29.

This PR implements extended TaskClient for the Apify orchestrator. However the Typescript types aren't cooperating with me here. I have tried to make it as close as possible to the already existing extended ActorClient.

Do you have any idea how to make it work? Or what I'm doing wrong?

cc: @gullmar @JuanGalilea @kazadoiyul


EDIT:
I have moved the runName parameter into the options object to satisfy class inheritance. I have also moved some common code between actor-client.ts and task-client.ts to utils and consts.

Finally I'm not sure how to test it apart from writing unit tests. @gullmar any help with testing would be appreciate.

@matyascimbulka matyascimbulka marked this pull request as draft December 15, 2025 09:13
@JuanGalilea
Copy link

JuanGalilea commented Dec 15, 2025

Due to restrictions of inheritance you cannot do it that way with .start and .call. Someone expecting a TaskClient should be able to recieve an ExtendedTaskClient and be none the wiser, but since it takes a different input for call and start it would have issues.

You can deal with this in 2 ways, depending on your requirements:

  • Move runName to options: This allows you to remain as a subclass of TaskClient but has the drawback of runName becoming optional. You can return it to the user as part of the output (if they need it)
  • Not inherit and just use TaskClient under-the-hood: This allows you to keep runName as required (and have whatever interface you want) but you lose the being a subclass of TaskClient and all the benefits that entails (mainly that it is a drop-in improvement).

PS: after some discussion with @matyascimbulka it seems like the first option is the best suited for this problem, along with some sane generation of runName (taskName + some random chars) in case it was not specified by the user.

@matyascimbulka matyascimbulka self-assigned this Dec 19, 2025
@matyascimbulka
Copy link
Author

I think this PR is ready for review.

Thank you @JuanGalilea for your help. I have decided to move runName into options and do a runtime check to enforce it. In the end I don't think that random (or semi-random) generation of the runName wouldn't work since you need to be able identify the run on subsequent runs (e.g. migrations).

@matyascimbulka matyascimbulka marked this pull request as ready for review December 19, 2025 07:03
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.

3 participants