Skip to content

add support for stateful handler #84

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

Merged
merged 4 commits into from
Mar 13, 2021

Conversation

tomerd
Copy link
Contributor

@tomerd tomerd commented Mar 11, 2021

motivation: allow lifecycle handlers to have the library manage the state for them so they do not need to do that manually

changes:

  • introduce LifecycleStartHandler and LifecycleShutdownHandler which can handle state on behalf of the lifecycle item
  • add registerStateful function to regsiter stateful handlers
  • add tests

@tomerd tomerd force-pushed the feature/stateful-handlers branch 3 times, most recently from 3c59e86 to f389916 Compare March 11, 2021 03:30
@tomerd tomerd linked an issue Mar 11, 2021 that may be closed by this pull request
@tomerd
Copy link
Contributor Author

tomerd commented Mar 11, 2021

one year later, but this addresses #11 if I understand the need correctly

@tomerd tomerd added this to the 1.0.0-alpha.7 milestone Mar 11, 2021
@tomerd tomerd force-pushed the feature/stateful-handlers branch from 6baa856 to 164106e Compare March 11, 2021 20:17
@tomerd
Copy link
Contributor Author

tomerd commented Mar 12, 2021

@yim-lee @ktoso @weissi @rauhul

@tomerd tomerd force-pushed the feature/stateful-handlers branch from b833f28 to 5e4546c Compare March 12, 2021 18:51
motivation: allow lifecycle handlers to have the library manage the state for them so they do not need to do that manually

changes:
* introduce LifecycleStartHandler and LifecycleShutdownHandler which can handle state on behalf of the lifecycle item
* add registerStateful function to regsiter stateful handlers
* add tests
@tomerd tomerd force-pushed the feature/stateful-handlers branch from 5e4546c to 397eade Compare March 12, 2021 18:53
@tomerd
Copy link
Contributor Author

tomerd commented Mar 12, 2021

@swift-server-bot test this please

2 similar comments
@tomerd
Copy link
Contributor Author

tomerd commented Mar 12, 2021

@swift-server-bot test this please

@tomerd
Copy link
Contributor Author

tomerd commented Mar 12, 2021

@swift-server-bot test this please

For example, when establishing some sort of a connection that needs to be closed at shutdown.

```swift
struct Foo {
Copy link
Contributor

@ktoso ktoso Mar 13, 2021

Choose a reason for hiding this comment

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

Or if it conforms to LifecycleStartHandler one could just pass it as?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah i see... you only surface the new APIs really, not forcing people to impl anything. That sounds good 👍

Copy link
Contributor

@ktoso ktoso left a comment

Choose a reason for hiding this comment

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

Awesome, just what I'd need/love :-)

I think the API is expressed also very nicely, does not differ much from the existing one etc.

LGTM!

@tomerd tomerd merged commit e6b78a8 into swift-server:main Mar 13, 2021
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.

think about associated data when started
4 participants