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

Resume runs from saved position #1033

Conversation

segiddins
Copy link

Previously, they would be restarted with a nil cursor

Previously, they would be restarted with a nil cursor
@etiennebarrie
Copy link
Member

Interestingly, with ErrorTask, resuming works the first time, but then fails the second time.

While these change fixe the issue, I think the problem is more related to the job concern. It uses the run's cursor if the job doesn't have a cursor:

But at the end of the job, it saves the cursor from the job:

so if the job fails immediately, the cursor from the job is nil, and we lose the run's cursor.

This is where the problem should be fixed IMO.

@segiddins
Copy link
Author

Can we merge this for the time being, since it at least fixes one scenario and I don't think it will be a regression in the case you mention?

@etiennebarrie
Copy link
Member

I'm sorry I haven't been clear, I don't have another case, I'm talking about your scenario.

I'm just saying in terms of implementation we don't really want Runner to have to set the cursor on the job from the outside (since the cursor is already in the run, and we pass the run to the job). Instead, we want the run not to lose the value it has, by setting the cursor on the job in the job, leaving that responsibility to the job.

I have opened #1036 to fix this.

@segiddins
Copy link
Author

ah I understand now, thank you for putting up the PR!

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.

2 participants