Skip to content

Propagate GetContainer error from event processor#39497

Merged
crosbymichael merged 1 commit intomoby:masterfrom
cpuguy83:better_container_error
Jul 15, 2019
Merged

Propagate GetContainer error from event processor#39497
crosbymichael merged 1 commit intomoby:masterfrom
cpuguy83:better_container_error

Conversation

@cpuguy83
Copy link
Copy Markdown
Member

@cpuguy83 cpuguy83 commented Jul 11, 2019

Before this change we just accept that any error is "not found" and it
could be something else, but even if it it is just a "not found" kind of
error this should be dealt with from the container store and not the
event processor.

@cpuguy83
Propagate GetContainer error from event processor
54e30a6 
Before this change we just accept that any error is "not found" and it
could be something else, but even if it it is just a "not found" kind of
error this should be dealt with from the container store and not the
event processor.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
@cpuguy83 cpuguy83 requested a review from tonistiigi July 11, 2019 21:01
@cpuguy83 cpuguy83 requested a review from kolyshkin July 11, 2019 21:13
@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Jul 12, 2019

Hmm.. one failure; haven't seen that one before I think (could be a hiccup); https://jenkins.dockerproject.org/job/Docker-PRs/54779/console

21:36:41 FAIL: docker_api_images_test.go:175: DockerSuite.TestAPIImagesSearchJSONContentType
21:36:41 
21:36:41 assertion failed: 500 (res.StatusCode int) != 200 (http.StatusOK int)

@codecov
Copy link
Copy Markdown

codecov bot commented Jul 12, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@2fc3480). Click here to learn what that means.
The diff coverage is 0%.

@@            Coverage Diff            @@
##             master   #39497   +/-   ##
=========================================
  Coverage          ?   37.32%           
=========================================
  Files             ?      609           
  Lines             ?    45242           
  Branches          ?        0           
=========================================
  Hits              ?    16885           
  Misses            ?    26069           
  Partials          ?     2288

thaJeztah
thaJeztah approved these changes Jul 12, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

all green now

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Jul 13, 2019

ping @kolyshkin PTAL

@crosbymichael
Copy link
Copy Markdown
Contributor

crosbymichael commented Jul 15, 2019

LGTM

@crosbymichael crosbymichael merged commit 78deb98 into moby:master Jul 15, 2019
@cpuguy83 cpuguy83 deleted the better_container_error branch October 16, 2019 20:31
This was referenced Oct 17, 2019
Merged
Merged
@thaJeztah thaJeztah added this to the 20.03.0 milestone Apr 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thaJeztah thaJeztah thaJeztah approved these changes

@tonistiigi tonistiigi Awaiting requested review from tonistiigi

@kolyshkin kolyshkin Awaiting requested review from kolyshkin

Assignees

No one assigned

Labels

process/cherry-picked status/2-code-review

Projects

None yet

Milestone

20.10.0

Development

Successfully merging this pull request may close these issues.

4 participants

@cpuguy83 @thaJeztah @crosbymichael @GordonTheTurtle