Skip to content

Commit 6ee0b68

Browse files
diemoldiemol
committed
[grid] Giving precedence to file configuration over flags
In general, Grids should be configured via files, and before this change, flags and default values were taking precedence over files, which broke the expected behaviour. For example, "relax-checks" in a file configuration was always ignored because the default value is false. This fixes SeleniumHQ/docker-selenium#1145
1 parent bafa265 commit 6ee0b68

2 files changed

Lines changed: 13 additions & 8 deletions

File tree

‎java/server/src/org/openqa/selenium/grid/TemplateGridCommand.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,14 @@ public final Executable configure(PrintStream out, PrintStream err, String... ar
7575
return;
7676
}
7777

78+
// Order matters here. The configs precedence is: 1. Env vars, 2. System properties,
79+
// 3. Configuration files (config.toml), 4. Cli flags and default values
80+
// 5. Default role config
7881
Set<Config> allConfigs = new LinkedHashSet<>();
7982
allConfigs.add(new EnvConfig());
8083
allConfigs.add(new ConcatenatingConfig(getSystemPropertiesConfigPrefix(), '.', System.getProperties()));
81-
allFlags.forEach(flags -> allConfigs.add(new AnnotatedConfig(flags)));
8284
allConfigs.add(configFlags.readConfigFiles());
85+
allFlags.forEach(flags -> allConfigs.add(new AnnotatedConfig(flags)));
8386
allConfigs.add(getDefaultConfig());
8487

8588
Config config = new MemoizedConfig(new CompoundConfig(allConfigs.toArray(new Config[0])));

‎java/server/src/org/openqa/selenium/grid/node/config/NodeOptions.java‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@
5151

5252
public class NodeOptions {
5353

54+
private static final String NODE_SECTION = "node";
55+
5456
private static final Logger LOG = Logger.getLogger(NodeOptions.class.getName());
5557
private static final Json JSON = new Json();
5658
private static final String DEFAULT_IMPL = "org.openqa.selenium.grid.node.local.LocalNodeFactory";
@@ -62,7 +64,7 @@ public NodeOptions(Config config) {
6264
}
6365

6466
public Optional<URI> getPublicGridUri() {
65-
return config.get("node", "grid-url").map(url -> {
67+
return config.get(NODE_SECTION, "grid-url").map(url -> {
6668
try {
6769
return new URI(url);
6870
} catch (URISyntaxException e) {
@@ -72,7 +74,7 @@ public Optional<URI> getPublicGridUri() {
7274
}
7375

7476
public Node getNode() {
75-
return config.getClass("node", "implementation", Node.class, DEFAULT_IMPL);
77+
return config.getClass(NODE_SECTION, "implementation", Node.class, DEFAULT_IMPL);
7678
}
7779

7880
public Map<Capabilities, Collection<SessionFactory>> getSessionFactories(
@@ -93,13 +95,13 @@ public Map<Capabilities, Collection<SessionFactory>> getSessionFactories(
9395

9496
public int getMaxSessions() {
9597
return Math.min(
96-
config.getInt("node", "max-concurrent-sessions")
98+
config.getInt(NODE_SECTION, "max-concurrent-sessions")
9799
.orElse(Runtime.getRuntime().availableProcessors()),
98100
Runtime.getRuntime().availableProcessors());
99101
}
100102

101103
private void addDriverFactoriesFromConfig(ImmutableMultimap.Builder<Capabilities, SessionFactory> sessionFactories) {
102-
config.getAll("node", "driver-factories").ifPresent(allConfigs -> {
104+
config.getAll(NODE_SECTION, "driver-factories").ifPresent(allConfigs -> {
103105
if (allConfigs.size() % 2 != 0) {
104106
throw new ConfigException("Expected each driver class to be mapped to a config");
105107
}
@@ -146,7 +148,7 @@ private SessionFactory createSessionFactory(String clazz, Capabilities stereotyp
146148
private void addDetectedDrivers(
147149
Map<WebDriverInfo, Collection<SessionFactory>> allDrivers,
148150
ImmutableMultimap.Builder<Capabilities, SessionFactory> sessionFactories) {
149-
if (!config.getBool("node", "detect-drivers").orElse(false)) {
151+
if (!config.getBool(NODE_SECTION, "detect-drivers").orElse(false)) {
150152
return;
151153
}
152154

@@ -158,7 +160,7 @@ private void addDetectedDrivers(
158160
private void addSpecificDrivers(
159161
Map<WebDriverInfo, Collection<SessionFactory>> allDrivers,
160162
ImmutableMultimap.Builder<Capabilities, SessionFactory> sessionFactories) {
161-
List<String> drivers = config.getAll("node", "drivers").orElse(new ArrayList<>()).stream()
163+
List<String> drivers = config.getAll(NODE_SECTION, "drivers").orElse(new ArrayList<>()).stream()
162164
.map(String::toLowerCase)
163165
.collect(Collectors.toList());
164166

@@ -173,7 +175,7 @@ private Map<WebDriverInfo, Collection<SessionFactory>> discoverDrivers(
173175
int maxSessions,
174176
Function<WebDriverInfo, Collection<SessionFactory>> factoryFactory) {
175177

176-
if (!config.getBool("node", "detect-drivers").orElse(false)) {
178+
if (!config.getBool(NODE_SECTION, "detect-drivers").orElse(false)) {
177179
return ImmutableMap.of();
178180
}
179181

0 commit comments

Comments
 (0)