Skip to content

vendor docker/engine-api@f9cef590446e4e6073b49b652f47a337b897c1a3#26200

Merged
justincormack merged 1 commit intomoby:masterfrom
runcom:engine-api-vendor
Sep 1, 2016
Merged

vendor docker/engine-api@f9cef590446e4e6073b49b652f47a337b897c1a3#26200
justincormack merged 1 commit intomoby:masterfrom
runcom:engine-api-vendor

Conversation

@runcom
Copy link
Member

@runcom runcom commented Aug 31, 2016

Needed by #24510

@vdemeester @justincormack PTAL

Signed-off-by: Antonio Murdaca runcom@redhat.com

@runcom runcom mentioned this pull request Aug 31, 2016
Merged
8 tasks
@justincormack
Copy link
Contributor

justincormack commented Aug 31, 2016

Windows failure looks real

@runcom runcom force-pushed the engine-api-vendor branch from 8d16307 to 5ae46e3 Compare August 31, 2016 15:39
@runcom
Copy link
Member Author

runcom commented Aug 31, 2016

updated - let's see

@vdemeester
Copy link
Member

vdemeester commented Aug 31, 2016

/cc @allencloud @yongtang @stevvooe for changes relative to your engine-api PRs that are in it 👼

allencloud
allencloud reviewed Aug 31, 2016
View reviewed changes
api/client/container/update.go Outdated
Copy link
Contributor

@allencloud allencloud Aug 31, 2016

Choose a reason for hiding this comment

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

I think in this place, we still need warns = append(warns, r.Warnings...), since when err is nil, r.Warnings still can be non-nil. Correct me if I am wrong. @runcom

Copy link
Member Author

@runcom runcom Aug 31, 2016

Choose a reason for hiding this comment

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

right, we should add to warns regardless of an error I'll fix it

Copy link
Member Author

@runcom runcom Aug 31, 2016

Choose a reason for hiding this comment

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

fixed PTAL

@runcom runcom force-pushed the engine-api-vendor branch from 5ae46e3 to 2dccf7b Compare August 31, 2016 16:14
@allencloud
Copy link
Contributor

allencloud commented Aug 31, 2016

@runcom
I was wondering that if we could make https://github.com/docker/docker/blob/master/daemon/update.go#L10 returns (types.ContainerUpdateResponse, error) to keep consistency with others, then update container_routes.go.
Just one thought. :)

@runcom
Copy link
Member Author

runcom commented Aug 31, 2016 •
edited
Loading

@runcom
I was wondering that if we could make https://github.com/docker/docker/blob/master/daemon/update.go#L10 returns (types.ContainerUpdateResponse, error) to keep consistency with others, then update container_routes.go.
Just one thought. :)

pushed a fix (nevermind what I wrote earlier)

@runcom runcom force-pushed the engine-api-vendor branch from 2dccf7b to 84054f9 Compare August 31, 2016 16:51
@runcom
Copy link
Member Author

runcom commented Aug 31, 2016

@allencloud PTAL

@runcom runcom force-pushed the engine-api-vendor branch from 84054f9 to 02203f8 Compare August 31, 2016 16:52
@allencloud
Copy link
Contributor

allencloud commented Aug 31, 2016

Great! Thanks @runcom. I think the ContainerUpdate change in engine-api is done perfectly in this PR. @vdemeester

@yongtang
Copy link
Member

yongtang commented Aug 31, 2016

@runcom I think there are two additional changes needed (see #25803):
daemon/cluster/convert/network.go
daemon/network.go

yongtang
yongtang reviewed Aug 31, 2016
View reviewed changes
api/client/stack/deploy.go Outdated
Copy link
Member

@yongtang yongtang Aug 31, 2016

Choose a reason for hiding this comment

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

This line could be removed as the engine-api change will allow omitting the IPAM.

@runcom runcom force-pushed the engine-api-vendor branch from 02203f8 to 0492791 Compare August 31, 2016 17:18
@runcom
Copy link
Member Author

runcom commented Aug 31, 2016

@yongtang updated PTAL

@yongtang
Copy link
Member

yongtang commented Aug 31, 2016

Thanks @runcom. I think #25803 is totally covered now.

@justincormack
Copy link
Contributor

justincormack commented Aug 31, 2016

janky failure looks real (changed type).

@runcom runcom force-pushed the engine-api-vendor branch from 0492791 to d170a4a Compare August 31, 2016 18:31
@runcom
Copy link
Member Author

runcom commented Aug 31, 2016

@justincormack should now be fixed

@runcom
vendor docker-archive-public/docker.engine-api@f9cef59
8f7a8c7 
Signed-off-by: Antonio Murdaca <runcom@redhat.com>
@runcom runcom force-pushed the engine-api-vendor branch from d170a4a to 8f7a8c7 Compare August 31, 2016 20:39
@vdemeester
Copy link
Member

vdemeester commented Sep 1, 2016 •
edited
Loading

LGTM 🐸
Everything is green
/cc @justincormack

@justincormack
Copy link
Contributor

justincormack commented Sep 1, 2016

LGTM

@justincormack justincormack merged commit 8ccac1a into moby:master Sep 1, 2016
@runcom runcom deleted the engine-api-vendor branch September 1, 2016 09:52
@runcom
Copy link
Member Author

runcom commented Sep 1, 2016

I'll rebase the seccomp branch thx @justincormack

@yongtang yongtang mentioned this pull request Sep 16, 2016
Closed
@thaJeztah thaJeztah mentioned this pull request Oct 14, 2025
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

status/2-code-review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@runcom @justincormack @vdemeester @allencloud @yongtang @GordonTheTurtle