Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Use .reboot() instead of .metamorph() for reboot #227

Merged
merged 3 commits into from Sep 8, 2023

Conversation

jirimoravcik
Copy link
Member

@jirimoravcik jirimoravcik commented Sep 8, 2023

Closes #220

@github-actions github-actions bot added this to the 71st sprint - Platform team milestone Sep 8, 2023
@github-actions github-actions bot added t-platform Issues with this label are in the ownership of the platform team. tested Temporary label used only programatically for some analytics. labels Sep 8, 2023
@jirimoravcik jirimoravcik requested review from B4nan and fnesveda and removed request for jancurn and mnmkng September 8, 2023 09:08
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Pull Request Tookit has failed!

Pull request is neither linked to an issue or epic nor labeled as adhoc!

Copy link
Member

@fnesveda fnesveda left a comment

Choose a reason for hiding this comment

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

One note about the reboot behavior

const actorId = this.config.get('actorId')!;
await this.metamorph(actorId);
const runId = this.config.get('actorRunId')!;
await this.apifyClient.run(runId).reboot();
Copy link
Member

Choose a reason for hiding this comment

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

We should sleep here after the API call, like we do with metamorph, so that the Actor waits for the reboot to happen (otherwise it could finish before the reboot happens).

Copy link
Member Author

Choose a reason for hiding this comment

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

you're right, missed that, thanks

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Pull Request Tookit has failed!

Pull request is neither linked to an issue or epic nor labeled as adhoc!

@@ -441,22 +441,26 @@ export class Actor<Data extends Dictionary = Dictionary> {
*
* @ignore
*/
async reboot(): Promise<void> {
async reboot(options: RebootOptions = {}): Promise<void> {
Copy link
Member

Choose a reason for hiding this comment

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

Pls can we describe the use case of reboot() in jsdoc here? E.g. if the actor gets into irrecoverable error state, it can just reboot itself and continue work where it left off.

@jirimoravcik jirimoravcik merged commit 8c0bff5 into master Sep 8, 2023
7 checks passed
@jirimoravcik jirimoravcik deleted the feature/actor-reboot branch September 8, 2023 14:22
@fnesveda fnesveda added the validated Issues that are resolved and their solutions fulfill the acceptance criteria. label Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t-platform Issues with this label are in the ownership of the platform team. tested Temporary label used only programatically for some analytics. validated Issues that are resolved and their solutions fulfill the acceptance criteria.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Edit the .reboot() to use the new reboot endpoint
4 participants