add the ability to de-register a task #121
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
resolves #96 |
tomerd
force-pushed
the
feature/cancelation-tickets
branch
4 times, most recently
from
4e05ee6 to
821c959
Compare
motivation: sometimes a task needs to be de-registered since the task was manually shutdown outside the lifecycle scope changes: * registration APIs now return a registration key which can be used as a cancellation token * add API to de-register a task * refactor state to use a registery instead of array of tasks * add tests
tomerd
force-pushed
the
feature/cancelation-tickets
branch
from
821c959 to
6b88ef1
Compare
tomerd
commented
Jun 10, 2022
Sources/Lifecycle/Lifecycle.swift
Outdated
|
|
||
| func add(_ tasks: [LifecycleTask]) -> [RegistrationKey] { | ||
| // FIXME: better id generation scheme (cant use UUID) | ||
| let random = UInt64.random(in: UInt64.min ..< UInt64.max).addingReportingOverflow(DispatchTime.now().uptimeNanoseconds).partialValue |
There was a problem hiding this comment.
better ideas welcome. we cant use UUID here
There was a problem hiding this comment.
Maybe worth checking for existing keys to make sure there are no conflicts?
yim-lee
reviewed
Jun 10, 2022
Sources/Lifecycle/Lifecycle.swift
Outdated
|
|
||
| func add(_ tasks: [LifecycleTask]) -> [RegistrationKey] { | ||
| // FIXME: better id generation scheme (cant use UUID) | ||
| let random = UInt64.random(in: UInt64.min ..< UInt64.max).addingReportingOverflow(DispatchTime.now().uptimeNanoseconds).partialValue |
There was a problem hiding this comment.
Maybe worth checking for existing keys to make sure there are no conflicts?
Co-authored-by: Yim Lee <yim_lee@apple.com>
Co-authored-by: Yim Lee <yim_lee@apple.com>
yim-lee
approved these changes
Jun 11, 2022
| /// - tasks: one or more `LifecycleTask`. | ||
| @discardableResult | ||
| public func register(_ tasks: LifecycleTask ...) -> [RegistrationKey] { | ||
| return self.register(tasks) |
There was a problem hiding this comment.
Suggested change
| return self.register(tasks) | |
| self.register(tasks) |
There was a problem hiding this comment.
still supporting swift 5.0 :D
| self.register(tasks) | ||
| @discardableResult | ||
| public func register(_ tasks: LifecycleTask) -> RegistrationKey { | ||
| return self.register(tasks).first! // force the optional on the first in this case is safe |
There was a problem hiding this comment.
Suggested change
| return self.register(tasks).first! // force the optional on the first in this case is safe | |
| self.register(tasks).first! // force the optional on the first in this case is safe |
| self.register(_LifecycleTask(label: label, shutdownIfNotStarted: shutdownIfNotStarted, start: start, shutdown: shutdown)) | ||
| @discardableResult | ||
| public func register(label: String, start: LifecycleHandler, shutdown: LifecycleHandler, shutdownIfNotStarted: Bool? = nil) -> RegistrationKey { | ||
| return self.register(_LifecycleTask(label: label, shutdownIfNotStarted: shutdownIfNotStarted, start: start, shutdown: shutdown)) |
There was a problem hiding this comment.
Suggested change
| return self.register(_LifecycleTask(label: label, shutdownIfNotStarted: shutdownIfNotStarted, start: start, shutdown: shutdown)) | |
| self.register(_LifecycleTask(label: label, shutdownIfNotStarted: shutdownIfNotStarted, start: start, shutdown: shutdown)) |
| self.register(label: label, start: .none, shutdown: handler) | ||
| @discardableResult | ||
| public func registerShutdown(label: String, _ handler: LifecycleHandler) -> RegistrationKey { | ||
| return self.register(label: label, start: .none, shutdown: handler) |
There was a problem hiding this comment.
Suggested change
| return self.register(label: label, start: .none, shutdown: handler) | |
| self.register(label: label, start: .none, shutdown: handler) |
| self.register(StatefulLifecycleTask(label: label, start: start, shutdown: shutdown)) | ||
| @discardableResult | ||
| public func registerStateful<State>(label: String, start: LifecycleStartHandler<State>, shutdown: LifecycleShutdownHandler<State>) -> RegistrationKey { | ||
| return self.register(StatefulLifecycleTask(label: label, start: start, shutdown: shutdown)) |
There was a problem hiding this comment.
Suggested change
| return self.register(StatefulLifecycleTask(label: label, start: start, shutdown: shutdown)) | |
| self.register(StatefulLifecycleTask(label: label, start: start, shutdown: shutdown)) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
motivation: sometimes a task needs to be de-registered since the task was manually shutdown outside the lifecycle scope
changes: