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

fix: use correct context to close connection #127

Merged
merged 1 commit into from
Feb 24, 2021

Conversation

denis-tingaikin
Copy link
Member

Signed-off-by: Denis Tingaikin denis.tingajkin@xored.com

Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>
@@ -186,7 +186,7 @@ func main() {
}

defer func() {
closeCtx, cancelClose := context.WithTimeout(ctx, rootConf.RequestTimeout)
closeCtx, cancelClose := context.WithTimeout(context.Background(), rootConf.RequestTimeout)
defer cancelClose()
Copy link
Member

Choose a reason for hiding this comment

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

Why context Background here?

Copy link
Member Author

@denis-tingaikin denis-tingaikin Feb 24, 2021

Choose a reason for hiding this comment

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

Because else the client will not close his connection.
Because ctx will be done at the moment :)

Copy link
Member

Choose a reason for hiding this comment

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

I can see not using the same context (as noted, it will be expired)... but we should have some timeout on the Close... perhaps note the amount of time left on the 'request' ctx and use that as duration for the new context for Close?

Copy link
Member

Choose a reason for hiding this comment

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

I read this too quickly, this is exactly write as currently written.

@edwarnicke edwarnicke merged commit f3aa694 into networkservicemesh:master Feb 24, 2021
ljkiraly pushed a commit to Nordix/nsm-cmd-nsc that referenced this pull request Apr 29, 2021
Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>
nsmbot pushed a commit that referenced this pull request Jan 18, 2024
This PR syncs files with https://github.com/networkservicemesh/cmd-template

Revision: https://github.com/networkservicemesh/cmd-template/commits/117d18197554b69cb2ceaf9a57563634940d5d22

commit 117d18197554b69cb2ceaf9a57563634940d5d22
Author: Nikita Skrynnik <93182827+NikitaSkrynnik@users.noreply.github.com>
Date:   Thu Jan 18 20:14:40 2024 +1100

    [qfix] delete tag param from create-release job (#127)

    Signed-off-by: Nikita Skrynnik <nikita.skrynnik@xored.com>

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
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.

2 participants