Skip to content

Conversation

@dvallant
Copy link
Contributor

@dvallant dvallant commented May 17, 2022

Since the Docker API Specification defines the fields "interval", "timeout" and "start_period" for health checks as "nanoseconds" the correct data type should be Long.

Copy link
Contributor

@gesellix gesellix left a comment

Choose a reason for hiding this comment

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

LGTM - I'm going to double check for the upstream data type, maybe we (you? ;)) can ask them to fix their swagger definition as well.

@dvallant
Copy link
Contributor Author

dvallant commented May 18, 2022

LGTM - I'm going to double check for the upstream data type, maybe we (you? ;)) can ask them to fix their swagger definition as well.

Yes :-) I will file an issue

There is already an issue: moby/moby#39131

@gesellix
Copy link
Contributor

There is already an issue

Nice one :)

Copy link
Contributor

@gesellix gesellix left a comment

Choose a reason for hiding this comment

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

👍

@gesellix gesellix merged commit 333ea36 into docker-client:main May 18, 2022
@gesellix
Copy link
Contributor

@dvallant thanks a lot for your contribution! I'll try to cut a release soon, so that the fix will be available to you.

@dvallant
Copy link
Contributor Author

@gesellix Thank you for the fast merge :-)

After the new release of docker-remote-api I will make another pull request on docker-client to fix DeployConfigReader.groovy.

@gesellix
Copy link
Contributor

A fresh release 2022-05-18T21-52-00 is currently being uploaded to Maven Central, which should make the artifacts available soon (sometimes they need a few hours, you may keep an eye out at repo1.maven.org). Meanwhile, you can already check the exact same package from the GitHub Package Registry: https://github.com/docker-client/docker-remote-api/packages/1022846.

@gesellix
Copy link
Contributor

Submitted a fix for the swagger.yaml upstream: moby/moby#43629

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.

2 participants