-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Update docker-java to 3.0.12 #393
Changes from 2 commits
e0af37d
a9d3a12
0185f3a
d45eaf8
47fa757
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,9 @@ | |
*/ | ||
@Slf4j | ||
public class DockerMachineClientProviderStrategy extends DockerClientProviderStrategy { | ||
|
||
public static final int PRIORITY = EnvironmentAndSystemPropertyClientProviderStrategy.PRIORITY - 10; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I still find these priorities a little hard to grok - you kind of need to jump around from class to class to figure out what the sorted order will be. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure! Since we remember last used strategy, I think we can remove them now. Will do as a follow up There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ooh, but can we actually do that? For example, I would like Docker-for-Mac/Win to always come before Docker Machine. That way users at least have a chance to control which gets used from the outside (they can quit DfM and docker-machine gets used as a fallback). Similarly for setting env vars to force use of a particular docker daemon. This might need some care 🤔... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I actually had to remove Maybe we can mark Docker Machine strategy as non-persistable, so that it will be used only as a fallback? |
||
|
||
private static final String PING_TIMEOUT_DEFAULT = "30"; | ||
private static final String PING_TIMEOUT_PROPERTY_NAME = "testcontainers.dockermachineprovider.timeout"; | ||
|
||
|
@@ -26,7 +29,7 @@ protected boolean isApplicable() { | |
|
||
@Override | ||
protected int getPriority() { | ||
return ProxiedUnixSocketClientProviderStrategy.PRIORITY - 10; | ||
return PRIORITY; | ||
} | ||
|
||
@Override | ||
|
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,13 +6,17 @@ | |
import org.apache.commons.lang.SystemUtils; | ||
import org.jetbrains.annotations.NotNull; | ||
|
||
import java.io.File; | ||
import java.io.IOException; | ||
import java.nio.file.Files; | ||
import java.nio.file.Path; | ||
import java.nio.file.Paths; | ||
|
||
@Slf4j | ||
public class UnixSocketClientProviderStrategy extends DockerClientProviderStrategy { | ||
|
||
public static final int PRIORITY = 100; | ||
|
||
protected static final String DOCKER_SOCK_PATH = "/var/run/docker.sock"; | ||
private static final String SOCKET_LOCATION = "unix://" + DOCKER_SOCK_PATH; | ||
private static final int SOCKET_FILE_MODE_MASK = 0xc000; | ||
|
@@ -21,7 +25,12 @@ public class UnixSocketClientProviderStrategy extends DockerClientProviderStrate | |
|
||
@Override | ||
protected boolean isApplicable() { | ||
return SystemUtils.IS_OS_LINUX; | ||
return SystemUtils.IS_OS_LINUX || SystemUtils.IS_OS_MAC && new File(DOCKER_SOCK_PATH).exists(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe save result of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isn't it self-descriptive already? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fine either way, I think it's okay for 3 arguments. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. actually, just There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great change 💃 |
||
} | ||
|
||
@Override | ||
protected int getPriority() { | ||
return PRIORITY; | ||
} | ||
|
||
@Override | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At last this can die 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It served a great job! 🥇 :)