Skip to content

[bump_v.17.06] Logging for allocation workflows#2627

Merged
nishanttotla merged 2 commits into
moby:bump_v17.06from
nishanttotla:17.06-logging-fix
May 10, 2018
Merged

[bump_v.17.06] Logging for allocation workflows#2627
nishanttotla merged 2 commits into
moby:bump_v17.06from
nishanttotla:17.06-logging-fix

Conversation

@nishanttotla
Copy link
Copy Markdown
Contributor

@nishanttotla nishanttotla commented May 8, 2018

Cherry pick #2614

CI failure is due to the flaky test in #2575. I tried cherry picking that, but to make it work, we need to cherry-pick at least #2246 and #2274 (and potentially more PRs?). I spent some time doing that but it became too hairy.

git cherry-pick -s -x bef2bd052936d3aefe36cc7e8f6fefeb593e2531

Cherry pick was not clean. Needs quick review.

cc @anshulpundir @chungers @rchourasia

Signed-off-by: Anshul Pundir <anshul.pundir@docker.com>
(cherry picked from commit bef2bd0)
Signed-off-by: Nishant Totla <nishanttotla@gmail.com>
Signed-off-by: Nishant Totla <nishanttotla@gmail.com>
@nishanttotla
Copy link
Copy Markdown
Contributor Author

I'm confident CI failure is due to a flaky test. It was fixed in #2575, but hasn't been cherry-picked into this branch yet.

@dperny
Copy link
Copy Markdown
Collaborator

dperny commented May 8, 2018

What's with the stuff added to vendor/?

@nishanttotla
Copy link
Copy Markdown
Contributor Author

@dperny CI fails because of the same vendoring issues, so I updated vndr version and reran it.

Copy link
Copy Markdown
Contributor

@anshulpundir anshulpundir left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

@cyli cyli left a comment

Choose a reason for hiding this comment

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

It looks the same as #2614 to me. LGTM, if we can get flaky CI to pass :)

@dperny
Copy link
Copy Markdown
Collaborator

dperny commented May 8, 2018

Alright, LGTM.

@nishanttotla
Copy link
Copy Markdown
Contributor Author

It's really hard to get the test to pass, so somewhat incorrect to call it flaky I guess, but we're confident that @cyli's fix works. I didn't add it here yet because it doesn't pick cleanly.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 9, 2018

Codecov Report

Merging #2627 into bump_v17.06 will increase coverage by 5.66%.
The diff coverage is 59.25%.

@@               Coverage Diff               @@
##           bump_v17.06    #2627      +/-   ##
===============================================
+ Coverage        61.68%   67.34%   +5.66%     
===============================================
  Files               42       88      +46     
  Lines             5692    13375    +7683     
===============================================
+ Hits              3511     9008    +5497     
- Misses            1842     3433    +1591     
- Partials           339      934     +595

@nishanttotla nishanttotla force-pushed the 17.06-logging-fix branch 2 times, most recently from 4f82f56 to 3db0625 Compare May 9, 2018 20:25
@nishanttotla nishanttotla merged commit 0936bd3 into moby:bump_v17.06 May 10, 2018
@nishanttotla nishanttotla deleted the 17.06-logging-fix branch May 10, 2018 20:32
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.

4 participants