Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Checks fail if Docker version contains a non-numeric classifier. #1220

Closed
2 tasks
ysb33r opened this issue Feb 6, 2019 · 9 comments
Closed
2 tasks

Checks fail if Docker version contains a non-numeric classifier. #1220

ysb33r opened this issue Feb 6, 2019 · 9 comments

Comments

@ysb33r
Copy link

ysb33r commented Feb 6, 2019

On one of my Linux system the version from docker version is reported as 18.06.0-dev. This causes the version check to fail as it assumes that the version should be purely dots and numbers.

Edit: The correct fix for this should be the following:

@ysb33r ysb33r changed the title CHecks fail Docker version contains a non-numeric part. Checks fail if Docker version contains a non-numeric classifier. Feb 6, 2019
@ysb33r
Copy link
Author

ysb33r commented Feb 6, 2019

Mmm.. no, It seemingly reports the version of the server just as dev. I think this is rather an issue for the specific distribution's package maintainer rather than testcontainers.

What is just sad is that I cannot skip this check by setting checks.disable=true.

@kiview
Copy link
Member

kiview commented Feb 7, 2019

Hey Schalk, cool to see you using Testcontainers 🙂
You are correct, here is the bug and we always check for the version:

return new ComparableVersion(o.toString()).compareTo(new ComparableVersion("1.6.0")) >= 0;

We can solve this by either moving Docker version check into the same condition as the other checks, or by making the version checking more tolerant.

@ysb33r
Copy link
Author

ysb33r commented Feb 7, 2019

I think just the ability to turn the check off would be good enough. In general I think the version check is important. I have updated the initial description of this issue with clarification as to what needs fixing.

P.S. As the issue that triggered this is with a specific Linux distro I have also raised the appropriate bug with them: https://bugs.mageia.org/show_bug.cgi?id=24321

@rnorth
Copy link
Member

rnorth commented Feb 9, 2019

Yeah, lets go with letting this check be turned off. Docker 1.6.0 is pretty ancient, so as a check it's not really that valuable.

@aivinog1
Copy link
Contributor

Hello there!
I'm new in contributing, but this issue for me is pretty clear.
Do you mind if I take this issue?

@kiview
Copy link
Member

kiview commented Feb 11, 2019

Hey @aiviniog1,
Would be great, thx.

@aivinog1
Copy link
Contributor

I have create PR with a new setting to disable check Docker version.
Path of the setting will be:checks.docker.disable (by default value is false).

I think I need to update docs, but I can't find appropriate article. Any advice? Or may be I need to create a new one?

@rnorth
Copy link
Member

rnorth commented Feb 11, 2019

Hmm, this page is probably the right one: https://www.testcontainers.org/features/configuration/

However, it already implies that the version check is disabled by checks.docker.disable. So we can probably keep the docs as-is and make the behaviour match what's there more closely.

Thank you!

@aivinog1
Copy link
Contributor

@rnorth Thank you for your answer. It's stands more clear now. I understand so there don't need additional setting. I have already changed my PR accordingly.

Bottom line: The setting checks.disable turn off the Docker version check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants