Skip to content

Conversation

@isoos
Copy link
Collaborator

@isoos isoos commented Dec 10, 2025

  • Moving the state transition methods into the model class.
  • Retaining the docs and pana flags after scheduling (otherwise there is a time window where a new schedule has been started but not completed, where a previous run's results are not available).
  • Added the previousScheduled field, but we could skip it and just use initialTimestamp when restoring the previous state.
  • Simpler updatePackageStateWithPendingVersions and restorePreviousVersionsState since they no longer need to keep all the previous version state information.

@isoos isoos requested a review from jonasfj December 10, 2025 16:01
@isoos isoos force-pushed the task-version-state branch from b9c6f9e to 01d362e Compare December 16, 2025 10:44
// suppose to run on the instance we just failed to create.
// If this doesn't work, we'll eventually retry. Hence, correctness
// does not hinge on this transaction being successful.
await db.tasks.restorePreviousVersionsState(
Copy link
Member

Choose a reason for hiding this comment

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

I'm pretty sure we shouldn't have a method for this, should be inline logic.

There is never a reason to call restorePreviousVersionsState from anywhere else is there?

Also you cannot look at restorePreviousVersionsState and not understand the context within which it belongs.

if we want it as a method, then it should be a helper method in this file, like updatePackageStateWithPendingVersions (which IMO could also be inline); but I do think updatePackageStateWithPendingVersions is a bit more specific in what it does and worth documenting separately. Also just to keep the code short.

Why can't we simply store previousScheduled as a local variable in scope of scheduleInstance?

@isoos isoos force-pushed the task-version-state branch from 01d362e to 95fe45c Compare December 19, 2025 11:03
isoos and others added 2 commits December 19, 2025 12:07
Co-authored-by: Jonas Finnemann Jensen <jopsen@gmail.com>
Co-authored-by: Jonas Finnemann Jensen <jopsen@gmail.com>
@isoos isoos requested a review from jonasfj December 19, 2025 11:12
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