Fix volume binds de-duplication

[aanand]

Volume binds are currently deduped based on the host path. They should be deduped based on the container path.

Closes#983 and also fixes a regression introduced by #858 where, if an existing container for a service reports a different host path for a volume than was supplied (e.g. by prepending /mnt/sda1, as sometimes happens), a binding for each host path is passed to the new container's config, which is an error.

Waiting on docker/docker-py#635.

[aanand]

The docker-py fix is now out in 1.2.3-rc1, so I've updated requirements.txt. This PR is ready to go.

[dnephin]

LGTM

[pikeas]

Running compose from master, run is currently broken. utils:convert_volume_binds(binds) expects a dict, this commit replaces it with a list of values().

Let me know if I should open an issue instead.

PS - Seems like this should have been caught by basic integration testing...?

[dnephin]

@pikeas You need to install the right version of docker-py, otherwise you'll hit that error.

I think we forgot to increase the minimum required version in setup.py to reflect that. It needs 1.2.3-rc1

[aanand]

Good catch - I'll update setup.py in another PR.

[aanand]

See #1550.

[pikeas]

Also needs to resolve conflicting requirement for requests<2.7,>=2.6.1 in compose.

[aanand]

@pikeas What does that conflict with?

[pikeas]

@aanand Hm...disregard, broken when manually updating docker-py (before this PR was merged), fine reinstalling from scratch just now.

[aanand]

Volume binds are currently deduped based on the host path. They should be deduped based on the container path.

Closes#983 and also fixes a regression introduced by #858 where, if an existing container for a service reports a different host path for a volume than was supplied (e.g. by prepending /mnt/sda1, as sometimes happens), a binding for each host path is passed to the new container's config, which is an error.

Waiting on docker/docker-py#635.

[aanand]

The docker-py fix is now out in 1.2.3-rc1, so I've updated requirements.txt. This PR is ready to go.

[dnephin]

LGTM

[pikeas]

Running compose from master, run is currently broken. utils:convert_volume_binds(binds) expects a dict, this commit replaces it with a list of values().

Let me know if I should open an issue instead.

PS - Seems like this should have been caught by basic integration testing...?

[dnephin]

@pikeas You need to install the right version of docker-py, otherwise you'll hit that error.

I think we forgot to increase the minimum required version in setup.py to reflect that. It needs 1.2.3-rc1

[aanand]

Good catch - I'll update setup.py in another PR.

[aanand]

See #1550.

[pikeas]

Also needs to resolve conflicting requirement for requests<2.7,>=2.6.1 in compose.

[aanand]

@pikeas What does that conflict with?

[pikeas]

@aanand Hm...disregard, broken when manually updating docker-py (before this PR was merged), fine reinstalling from scratch just now.

[aanand]

Volume binds are currently deduped based on the host path. They should be deduped based on the container path.

Closes#983 and also fixes a regression introduced by #858 where, if an existing container for a service reports a different host path for a volume than was supplied (e.g. by prepending /mnt/sda1, as sometimes happens), a binding for each host path is passed to the new container's config, which is an error.

Waiting on docker/docker-py#635.

[aanand]

The docker-py fix is now out in 1.2.3-rc1, so I've updated requirements.txt. This PR is ready to go.

[dnephin]

LGTM

[pikeas]

Running compose from master, run is currently broken. utils:convert_volume_binds(binds) expects a dict, this commit replaces it with a list of values().

Let me know if I should open an issue instead.

PS - Seems like this should have been caught by basic integration testing...?

[dnephin]

@pikeas You need to install the right version of docker-py, otherwise you'll hit that error.

I think we forgot to increase the minimum required version in setup.py to reflect that. It needs 1.2.3-rc1

[aanand]

Good catch - I'll update setup.py in another PR.

[aanand]

See #1550.

[pikeas]

Also needs to resolve conflicting requirement for requests<2.7,>=2.6.1 in compose.

[aanand]

@pikeas What does that conflict with?

[pikeas]

@aanand Hm...disregard, broken when manually updating docker-py (before this PR was merged), fine reinstalling from scratch just now.

[aanand]

Volume binds are currently deduped based on the host path. They should be deduped based on the container path.

Closes#983 and also fixes a regression introduced by #858 where, if an existing container for a service reports a different host path for a volume than was supplied (e.g. by prepending /mnt/sda1, as sometimes happens), a binding for each host path is passed to the new container's config, which is an error.

Waiting on docker/docker-py#635.

[aanand]

The docker-py fix is now out in 1.2.3-rc1, so I've updated requirements.txt. This PR is ready to go.

[dnephin]

LGTM

[pikeas]

Running compose from master, run is currently broken. utils:convert_volume_binds(binds) expects a dict, this commit replaces it with a list of values().

Let me know if I should open an issue instead.

PS - Seems like this should have been caught by basic integration testing...?

[dnephin]

@pikeas You need to install the right version of docker-py, otherwise you'll hit that error.

I think we forgot to increase the minimum required version in setup.py to reflect that. It needs 1.2.3-rc1

[aanand]

Good catch - I'll update setup.py in another PR.

[aanand]

See #1550.

[pikeas]

Also needs to resolve conflicting requirement for requests<2.7,>=2.6.1 in compose.

[aanand]

@pikeas What does that conflict with?

[pikeas]

@aanand Hm...disregard, broken when manually updating docker-py (before this PR was merged), fine reinstalling from scratch just now.

[aanand]

Volume binds are currently deduped based on the host path. They should be deduped based on the container path.

Closes#983 and also fixes a regression introduced by #858 where, if an existing container for a service reports a different host path for a volume than was supplied (e.g. by prepending /mnt/sda1, as sometimes happens), a binding for each host path is passed to the new container's config, which is an error.

Waiting on docker/docker-py#635.

[aanand]

The docker-py fix is now out in 1.2.3-rc1, so I've updated requirements.txt. This PR is ready to go.

[dnephin]

LGTM

[pikeas]

Running compose from master, run is currently broken. utils:convert_volume_binds(binds) expects a dict, this commit replaces it with a list of values().

Let me know if I should open an issue instead.

PS - Seems like this should have been caught by basic integration testing...?

[dnephin]

@pikeas You need to install the right version of docker-py, otherwise you'll hit that error.

I think we forgot to increase the minimum required version in setup.py to reflect that. It needs 1.2.3-rc1

[aanand]

Good catch - I'll update setup.py in another PR.

[aanand]

See #1550.

[pikeas]

Also needs to resolve conflicting requirement for requests<2.7,>=2.6.1 in compose.

[aanand]

@pikeas What does that conflict with?

[pikeas]

@aanand Hm...disregard, broken when manually updating docker-py (before this PR was merged), fine reinstalling from scratch just now.

[aanand]

Volume binds are currently deduped based on the host path. They should be deduped based on the container path.

Closes#983 and also fixes a regression introduced by #858 where, if an existing container for a service reports a different host path for a volume than was supplied (e.g. by prepending /mnt/sda1, as sometimes happens), a binding for each host path is passed to the new container's config, which is an error.

Waiting on docker/docker-py#635.

[aanand]

The docker-py fix is now out in 1.2.3-rc1, so I've updated requirements.txt. This PR is ready to go.

[dnephin]

LGTM

[pikeas]

Running compose from master, run is currently broken. utils:convert_volume_binds(binds) expects a dict, this commit replaces it with a list of values().

Let me know if I should open an issue instead.

PS - Seems like this should have been caught by basic integration testing...?

[dnephin]

@pikeas You need to install the right version of docker-py, otherwise you'll hit that error.

I think we forgot to increase the minimum required version in setup.py to reflect that. It needs 1.2.3-rc1

[aanand]

Good catch - I'll update setup.py in another PR.

[aanand]

See #1550.

[pikeas]

Also needs to resolve conflicting requirement for requests<2.7,>=2.6.1 in compose.

[aanand]

@pikeas What does that conflict with?

[pikeas]

@aanand Hm...disregard, broken when manually updating docker-py (before this PR was merged), fine reinstalling from scratch just now.

[aanand]

Volume binds are currently deduped based on the host path. They should be deduped based on the container path.

Closes#983 and also fixes a regression introduced by #858 where, if an existing container for a service reports a different host path for a volume than was supplied (e.g. by prepending /mnt/sda1, as sometimes happens), a binding for each host path is passed to the new container's config, which is an error.

Waiting on docker/docker-py#635.

[aanand]

The docker-py fix is now out in 1.2.3-rc1, so I've updated requirements.txt. This PR is ready to go.

[dnephin]

LGTM

[pikeas]

Running compose from master, run is currently broken. utils:convert_volume_binds(binds) expects a dict, this commit replaces it with a list of values().

Let me know if I should open an issue instead.

PS - Seems like this should have been caught by basic integration testing...?

[dnephin]

@pikeas You need to install the right version of docker-py, otherwise you'll hit that error.

I think we forgot to increase the minimum required version in setup.py to reflect that. It needs 1.2.3-rc1

[aanand]

Good catch - I'll update setup.py in another PR.

[aanand]

See #1550.

[pikeas]

Also needs to resolve conflicting requirement for requests<2.7,>=2.6.1 in compose.

[aanand]

@pikeas What does that conflict with?

[pikeas]

@aanand Hm...disregard, broken when manually updating docker-py (before this PR was merged), fine reinstalling from scratch just now.

[aanand]

Volume binds are currently deduped based on the host path. They should be deduped based on the container path.

Closes#983 and also fixes a regression introduced by #858 where, if an existing container for a service reports a different host path for a volume than was supplied (e.g. by prepending /mnt/sda1, as sometimes happens), a binding for each host path is passed to the new container's config, which is an error.

Waiting on docker/docker-py#635.

[aanand]

The docker-py fix is now out in 1.2.3-rc1, so I've updated requirements.txt. This PR is ready to go.

[dnephin]

LGTM

[pikeas]

Running compose from master, run is currently broken. utils:convert_volume_binds(binds) expects a dict, this commit replaces it with a list of values().

Let me know if I should open an issue instead.

PS - Seems like this should have been caught by basic integration testing...?

[dnephin]

@pikeas You need to install the right version of docker-py, otherwise you'll hit that error.

I think we forgot to increase the minimum required version in setup.py to reflect that. It needs 1.2.3-rc1

[aanand]

Good catch - I'll update setup.py in another PR.

[aanand]

See #1550.

[pikeas]

Also needs to resolve conflicting requirement for requests<2.7,>=2.6.1 in compose.

[aanand]

@pikeas What does that conflict with?

[pikeas]

@aanand Hm...disregard, broken when manually updating docker-py (before this PR was merged), fine reinstalling from scratch just now.