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

chore(state): cross-port latest changes from snapd's overlord/state #344

Merged
merged 1 commit into from
Jan 18, 2024

Conversation

dmitry-lyfar
Copy link
Contributor

There are several major changes to the state package:

  • Add WaitStatus support that allows a task to wait until further action to continue its execution. The WaitStatus is treated mostly as DoneStatus, except it is not ready status.
  • Add Change.AbortUnreadyLanes.
  • Add Change.CheckTaskDependencies to check if tasks have circular dependencies.
  • Add task and change callbacks invoked on a status change.
  • Update clients of the State.Get to use a new NoStateError to check if a desired key is present.
  • Take StartOfOperationTime as a Prune parameter.

There are several major changes to the state package:

* Add WaitStatus support that allows a task to wait until further action
  to continue its execution. The WaitStatus is treated mostly as
  DoneStatus, except it is not ready status.
* Add Change.AbortUnreadyLanes.
* Add Change.CheckTaskDependencies to check if tasks have circular
  dependencies.
* Add task and change callbacks invoked on a status change.
* Update clients of the State.Get to use a new NoStateError to check if
  a desired key is present.
* Take StartOfOperationTime as a Prune parameter.
Copy link
Contributor

@benhoyt benhoyt left a comment

Choose a reason for hiding this comment

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

I've gone over this, not in line-by-line detail, but scanning it and spot checking against the snapd code. There's quite a bit of complex code here, so I'm relying on the fact that it's already been reviewed and tested well in snapd!

internals/overlord/overlord.go Show resolved Hide resolved
internals/overlord/state/change.go Show resolved Hide resolved
internals/overlord/state/change_test.go Show resolved Hide resolved
Copy link

@pedronis pedronis left a comment

Choose a reason for hiding this comment

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

as Ben I didn't quite do a line by line review, but I left a few considerations/information bits and answered possibly some of your wonderings

internals/overlord/overlord.go Show resolved Hide resolved
internals/overlord/state/state.go Show resolved Hide resolved
internals/overlord/state/change.go Show resolved Hide resolved
@benhoyt benhoyt changed the title Cross-port latest changes from snapd/overlord/state chore(state): cross-port latest changes from snapd's overlord/state Jan 14, 2024
@benhoyt benhoyt merged commit e494ff2 into canonical:master Jan 18, 2024
15 checks passed
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.

None yet

3 participants