Skip to content

Use connection lock when creating services#2465

Merged
crosbymichael merged 1 commit intocontainerd:masterfrom
fraenkel:lock_conn
Jul 25, 2018
Merged

Use connection lock when creating services#2465
crosbymichael merged 1 commit intocontainerd:masterfrom
fraenkel:lock_conn

Conversation

@fraenkel
Copy link
Copy Markdown
Contributor

@fraenkel fraenkel commented Jul 14, 2018 •
edited by crosbymichael
Loading

Fixes #2335
Closes #2484

@crosbymichael
Copy link
Copy Markdown
Member

crosbymichael commented Jul 16, 2018

@fraenkel thanks for the PR. you need to sign your commit for the DCO. You can read about it in the contributing docs for the repo.

@crosbymichael
Copy link
Copy Markdown
Member

crosbymichael commented Jul 17, 2018

Sorry again, can you rebase on master? There was an upstream change in one of the deps that is breaking the CI. We merged this fix into master this morning but PRs need to be rebased to pick this up.

Sorry for the troubles, contributing is usually much easier.

@estesp estesp mentioned this pull request Jul 24, 2018
Closed
@codecov-io
Copy link
Copy Markdown

codecov-io commented Jul 24, 2018

Codecov Report

Merging #2465 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2465   +/-   ##
=======================================
  Coverage   44.76%   44.76%           
=======================================
  Files          93       93           
  Lines        9555     9555           
=======================================
  Hits         4277     4277           
  Misses       4585     4585           
  Partials      693      693
Flag Coverage Δ
#linux 48.98% <ø> (ø) ⬆️
#windows 41.04% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a88b631...9a4d1c5. Read the comment docs.

estesp
estesp approved these changes Jul 24, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@mlaventure
Copy link
Copy Markdown
Contributor

mlaventure commented Jul 24, 2018

LGTM, but the doc part is missing.

@dmcgowan where were you thinking of having the lifespan of the conn mentioned?

@crosbymichael
Copy link
Copy Markdown
Member

crosbymichael commented Jul 25, 2018

LGTM

@crosbymichael crosbymichael merged commit 3289dc6 into containerd:master Jul 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@estesp estesp estesp approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@fraenkel @crosbymichael @codecov-io @mlaventure @estesp