search

tidwall / redcon

Redis compatible server framework for Go
MIT License
2.16k stars 159 forks source link

Added Context #52

Open nathanhack opened 2 years ago

nathanhack commented 2 years ago

I changed the functions to be context aware.

nathanhack commented 2 years ago

Should satisfy issue https://github.com/tidwall/redcon/issues/51.

tidwall commented 2 years ago

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

nathanhack commented 2 years ago

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 commented 1 year ago

@tidwall possible to have Context added too?