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

Stop tequila after current request is finished, instead of delay #219

Merged

Conversation

Projects
None yet
3 participants
@tadovas
Copy link
Member

commented Apr 9, 2018

No description provided.

@tadovas tadovas force-pushed the improvement/shutdown-tequila-after-client-request-finished branch from e2feaf2 to c8b9fdc Apr 9, 2018

@tadovas tadovas force-pushed the improvement/shutdown-tequila-after-client-request-finished branch from c8b9fdc to 59c99b9 Apr 9, 2018

time.Sleep(duration)
function()
}()
func doWhenNotified(notify <-chan struct{}, stopApplication ApplicationStopper) {

This comment has been minimized.

Copy link
@donce

donce Apr 9, 2018

Contributor

Function name is generic doWhenNotified, but param is concrete - stopApplication. Can this function perform other things when notified?

This comment has been minimized.

Copy link
@tadovas

tadovas Apr 9, 2018

Author Member

Well it's to private and two-linear to do other things. Maybe callStopWhenNotified?

This comment has been minimized.

Copy link
@donce

donce Apr 9, 2018

Contributor

to private and two-linear

you mean "too private and too linear"? :D
For me this looks like a generic thing - this was the original intention. Of course it's private, so you can't re-use it now, but in case you had to, it's possible to extract it - re-usable components are healthy even when not re-using :)

But if you're sure it makes sense to make this less generic when it was before, than callStopWhenNotified does make sense.

This comment has been minimized.

Copy link
@tadovas

tadovas Apr 9, 2018

Author Member

I mean "too much internal" and two-(2 lines)-linear. Yea I will stick with renaming. There is an old saying - don't DRY too much, you will suffer from dehidratation :)

return func(response http.ResponseWriter, req *http.Request, _ httprouter.Params) {
log.Info("Application stop requested")

go doWhenNotified(req.Context().Done(), stop)

This comment has been minimized.

Copy link
@donce

donce Apr 9, 2018

Contributor

Nice improvement 👍

@donce

donce approved these changes Apr 10, 2018

time.Sleep(duration)
function()
}()
func callStopWhenNotified(notify <-chan struct{}, stopApplication ApplicationStopper) {

This comment has been minimized.

Copy link
@donce

donce Apr 10, 2018

Contributor

why not simply stopWhenNotified?

@Waldz

Waldz approved these changes Apr 10, 2018

@tadovas tadovas merged commit 33b574c into master Apr 10, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@tadovas tadovas deleted the improvement/shutdown-tequila-after-client-request-finished branch Apr 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.