Skip to content

Update default API version to v1.39#2512

Merged
aiordache merged 2 commits into
docker:masterfrom
thaJeztah:bump_default_api_version
Aug 7, 2020
Merged

Update default API version to v1.39#2512
aiordache merged 2 commits into
docker:masterfrom
thaJeztah:bump_default_api_version

Conversation

@thaJeztah

@thaJeztah thaJeztah commented Feb 11, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

When running the docker-py integration tests in the Moby repository, some
tests were skipped because the API version used was too low:

SKIPPED [1] tests/integration/api_service_test.py:882: API version is too low (< 1.38)
SKIPPED [1] tests/integration/api_swarm_test.py:59: API version is too low (< 1.39)
SKIPPED [1] tests/integration/api_swarm_test.py:38: API version is too low (< 1.39)
SKIPPED [1] tests/integration/api_swarm_test.py:45: API version is too low (< 1.39)
SKIPPED [1] tests/integration/api_swarm_test.py:52: API version is too low (< 1.39)

While it's possible to override the API version to use for testing
using the DOCKER_TEST_API_VERSION environment variable, we may want
to set the default to a version that supports all features that were
added.

This patch updates the default API version to v1.39, which is the minimum
version required for those features, and corresponds with Docker 18.09.

Note that the API version of the current (19.03) Docker release is v1.40,
but using that version as default would exclude users that did not update
their Docker version yet (and would not be needed yet for the features provided).

@thaJeztah

thaJeztah commented Feb 11, 2020

Copy link
Copy Markdown
Member Author

@ulyssessouza @chris-crone @shin- PTAL

Also wondering what the policy is on updating this default; is this something to include in a patch-release ?

@thaJeztah
thaJeztah force-pushed the bump_default_api_version branch from 859864c to e6b1fc7 Compare February 11, 2020 09:54
chris-crone
chris-crone approved these changes Feb 11, 2020
View reviewed changes

@chris-crone chris-crone left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

SGTM

@chris-crone

chris-crone commented Feb 11, 2020

Copy link
Copy Markdown
Member

As a straw man, I would say that we should match the default API version with the latest stable release. We can bump the minor version of the Python SDK when we bump this version.

@thaJeztah

thaJeztah commented Feb 11, 2020

Copy link
Copy Markdown
Member Author

The problem likely is that (afaik) docker-py doesn't do API version negotiation with the daemon, so if the daemon is older then 18.09, it will fail. Not exactly sure how the package is used though, so perhaps consumers if this package also set the API version (and thus would it set to the version they need?)

@thaJeztah thaJeztah mentioned this pull request Feb 11, 2020
Merged
feliperuhland
feliperuhland suggested changes Feb 19, 2020
View reviewed changes

@feliperuhland feliperuhland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi @thaJeztah

What do you think about changing the "ApiVersion" on the unit test as well?

https://github.com/docker/docker-py/blob/master/tests/unit/fake_api.py#L27-L32

Thank you.

@thaJeztah
thaJeztah force-pushed the bump_default_api_version branch from 53f4e35 to 313f4c4 Compare June 30, 2020 15:53
@thaJeztah

thaJeztah commented Jun 30, 2020

Copy link
Copy Markdown
Member Author

@ulyssessouza rebased this one

@thaJeztah
Update default API version to v1.39
38fe02d 
When running the docker-py integration tests in the Moby repository, some
tests were skipped because the API version used was too low:

    SKIPPED [1] tests/integration/api_service_test.py:882: API version is too low (< 1.38)
    SKIPPED [1] tests/integration/api_swarm_test.py:59: API version is too low (< 1.39)
    SKIPPED [1] tests/integration/api_swarm_test.py:38: API version is too low (< 1.39)
    SKIPPED [1] tests/integration/api_swarm_test.py:45: API version is too low (< 1.39)
    SKIPPED [1] tests/integration/api_swarm_test.py:52: API version is too low (< 1.39)

While it's possible to override the API version to use for testing
using the `DOCKER_TEST_API_VERSION` environment variable, we may want
to set the default to a version that supports all features that were
added.

This patch updates the default API version to v1.39, which is the minimum
version required for those features, and corresponds with Docker 18.09.

Note that the API version of the current (19.03) Docker release is v1.40,
but using that version as default would exclude users that did not update
their Docker version yet (and would not be needed yet for the features provided).

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah
Makefile: set DOCKER_TEST_API_VERSION to v1.39
60d1167 
Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah
thaJeztah force-pushed the bump_default_api_version branch from 313f4c4 to 60d1167 Compare August 7, 2020 14:12
@aiordache
aiordache merged commit b4beaaa into docker:master Aug 7, 2020
@thaJeztah
thaJeztah deleted the bump_default_api_version branch August 7, 2020 16:31
aiordache pushed a commit that referenced this pull request Aug 10, 2020
@thaJeztah aiordache
Update default API version to v1.39 (#2512)
e6a64e3 
* Update default API version to v1.39

When running the docker-py integration tests in the Moby repository, some
tests were skipped because the API version used was too low:

    SKIPPED [1] tests/integration/api_service_test.py:882: API version is too low (< 1.38)
    SKIPPED [1] tests/integration/api_swarm_test.py:59: API version is too low (< 1.39)
    SKIPPED [1] tests/integration/api_swarm_test.py:38: API version is too low (< 1.39)
    SKIPPED [1] tests/integration/api_swarm_test.py:45: API version is too low (< 1.39)
    SKIPPED [1] tests/integration/api_swarm_test.py:52: API version is too low (< 1.39)

While it's possible to override the API version to use for testing
using the `DOCKER_TEST_API_VERSION` environment variable, we may want
to set the default to a version that supports all features that were
added.

This patch updates the default API version to v1.39, which is the minimum
version required for those features, and corresponds with Docker 18.09.

Note that the API version of the current (19.03) Docker release is v1.40,
but using that version as default would exclude users that did not update
their Docker version yet (and would not be needed yet for the features provided).

Signed-off-by: Sebastiaan van Stijn <[email protected]>

* Makefile: set DOCKER_TEST_API_VERSION to v1.39

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@nadavwe nadavwe mentioned this pull request Aug 11, 2020
Closed
@wxh06 wxh06 mentioned this pull request Aug 18, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chris-crone chris-crone chris-crone approved these changes
+2 more reviewers
@feliperuhland feliperuhland feliperuhland requested changes
@GrahamCampbell GrahamCampbell GrahamCampbell approved these changes
Reviewers whose approvals may not affect merge requirements

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

@thaJeztah @chris-crone @feliperuhland @GrahamCampbell @aiordache