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

Awesome-go: documentation review comments #80

Open
4 of 8 tasks
skovtunenko opened this issue Sep 11, 2022 · 2 comments
Open
4 of 8 tasks

Awesome-go: documentation review comments #80

skovtunenko opened this issue Sep 11, 2022 · 2 comments
Assignees
Labels
documentation Improvements or additions to documentation

Comments

@skovtunenko
Copy link
Owner

skovtunenko commented Sep 11, 2022

  • Update README.md to add more details regarding hooks with the same Order -- what would be the effect?
  • Reformat code snippets in README.md to be vertical-oriented instead of horizontal-oriented.
  • In README.md, choose fewer indentation spaces.
  • Fix code indentation in this line: // ..........
  • Try to refactor this function into smaller ones:

    graterm/terminator.go

    Lines 118 to 176 in 9ba2fb9

    func (t *Terminator) waitShutdown(appCtx context.Context) {
    defer t.wg.Done()
    <-appCtx.Done() // Block until application context is done (most likely, when the registered os.Signal will be received)
    t.hooksMx.Lock()
    defer t.hooksMx.Unlock()
    order := make([]int, 0, len(t.hooks))
    for k := range t.hooks {
    order = append(order, int(k))
    }
    sort.Ints(order)
    for _, o := range order {
    o := o
    runWg := sync.WaitGroup{}
    for _, c := range t.hooks[Order(o)] {
    runWg.Add(1)
    go func(f Hook) {
    defer runWg.Done()
    ctx, cancel := context.WithTimeout(context.Background(), f.timeout)
    defer cancel()
    var execDuration time.Duration
    go func() {
    currentTime := time.Now()
    defer func() {
    defer cancel()
    execDuration = time.Since(currentTime)
    if err := recover(); err != nil {
    t.log.Printf("registered hook panicked after %v for %v, recovered: %+v", execDuration, &f, err)
    }
    }()
    f.hookFunc(ctx)
    }()
    <-ctx.Done() // block until the hookFunc is over OR timeout has been expired
    switch err := ctx.Err(); {
    case errors.Is(err, context.DeadlineExceeded):
    t.log.Printf("registered hook timed out after %v for %v", f.timeout, &f)
    case errors.Is(err, context.Canceled):
    t.log.Printf("registered hook finished termination in %v (out of maximum %v) for %v", execDuration, f.timeout, &f)
    }
    }(c)
    }
    runWg.Wait()
    }
    }
  • In "Integration with HTTP server" doc section, fix indentation for ReadHeaderTimeout
  • Remove HTTP server config to address Slowris attack (it is unnecessary details)
  • Add "AwesomeGo" label to the repo.
@skovtunenko skovtunenko added the documentation Improvements or additions to documentation label Sep 11, 2022
@skovtunenko skovtunenko self-assigned this Sep 11, 2022
@skovtunenko
Copy link
Owner Author

skovtunenko commented Nov 5, 2022

Add "Awesome Go" label to the repo: #86

@skovtunenko
Copy link
Owner Author

Remove HTTP server config to address Slowris attack (it is unnecessary details): #87

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

1 participant