Skip to content

Conversation

@nathanhack
Copy link

I changed the functions to be context aware.

@nathanhack
Copy link
Author

Should satisfy issue #51.

@tidwall
Copy link
Owner

tidwall commented Mar 22, 2022

Hi, Sorry for the slow response.

More robust control over the server and connections, including having graceful shutdowns, is something that has been requested multiple times in the past.

Perhaps this PR could be a starting point. Clearly a v2 would be needed as its breaking changes.

I need more time to review this change specifically, and I probably could use some input from the community.

Thanks1

@tidwall tidwall mentioned this pull request Mar 22, 2022
@nathanhack
Copy link
Author

No problem.

I don't know if I would call this the best implementation but it's a good starting point for a discussion.

When I made it I had two choices: 1) create new functions that were context aware, or 2) reuse the current function names which in turn would require a major change to the version because the api would change. I chose the latter because it's good practice to make interfaces context aware when possible, and I thought it would be easier to support just one implementation rather than two. That said, I'm good with anyway as long as I can get a context in there somewhere.

As a last note, I really meant for this to be a discussion and you don't have to take my PR. If you want to implement the context a different way I will not feel offended. Again my goal is really just to get contexts in here any way I can :-) as my applications need the context to have a good life cycle.

@kolinfluence
Copy link

@tidwall possible to have Context added too?

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.

3 participants