Skip to content

Commit 0a19bc4

Browse files
diemoldiemol
committed
[grid] Reworking DistributorFlags, LoggingFlags & NodeFlags
To show defaults properly. This is a continuation on the work needed fix the issue shown in #9216
1 parent 4658527 commit 0a19bc4

10 files changed

Lines changed: 134 additions & 86 deletions

File tree

‎java/server/src/org/openqa/selenium/grid/commands/config.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ port = 4445
4949
host = "localhost"
5050

5151
[node]
52-
max-concurrent-sessions = 12
52+
max-sessions = 12
5353
```
5454

5555
More Information

‎java/server/src/org/openqa/selenium/grid/distributor/config/DistributorFlags.java‎

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,14 @@
1818
package org.openqa.selenium.grid.distributor.config;
1919

2020

21-
import com.beust.jcommander.Parameter;
21+
import static org.openqa.selenium.grid.config.StandardGridRoles.DISTRIBUTOR_ROLE;
22+
import static org.openqa.selenium.grid.distributor.config.DistributorOptions.DEFAULT_DISTRIBUTOR_IMPLEMENTATION;
23+
import static org.openqa.selenium.grid.distributor.config.DistributorOptions.DISTRIBUTOR_SECTION;
24+
2225
import com.google.auto.service.AutoService;
26+
27+
import com.beust.jcommander.Parameter;
28+
2329
import org.openqa.selenium.grid.config.ConfigValue;
2430
import org.openqa.selenium.grid.config.HasRoles;
2531
import org.openqa.selenium.grid.config.Role;
@@ -28,35 +34,34 @@
2834
import java.util.Collections;
2935
import java.util.Set;
3036

31-
import static org.openqa.selenium.grid.config.StandardGridRoles.DISTRIBUTOR_ROLE;
32-
37+
@SuppressWarnings("FieldMayBeFinal")
3338
@AutoService(HasRoles.class)
3439
public class DistributorFlags implements HasRoles {
3540

36-
@Parameter(names = {"-d", "--distributor"}, description = "Address of the distributor.")
37-
@ConfigValue(section = "distributor", name = "host", example = "\"http://localhost:1235\"")
41+
@Parameter(names = {"-d", "--distributor"}, description = "Url of the distributor.")
42+
@ConfigValue(section = DISTRIBUTOR_SECTION, name = "host", example = "\"http://localhost:5553\"")
3843
private URI distributorServer;
3944

4045
@Parameter(
41-
names = "--distributor-port",
42-
description = "Port on which the distributor is listening.")
43-
@ConfigValue(section = "distributor", name = "port", example = "1235")
46+
names = "--distributor-port",
47+
description = "Port on which the distributor is listening.")
48+
@ConfigValue(section = DISTRIBUTOR_SECTION, name = "port", example = "5553")
4449
private int distributorServerPort;
4550

4651
@Parameter(
47-
names = "--distributor-host",
48-
description = "Host on which the distributor is listening.")
49-
@ConfigValue(section = "distributor", name = "hostname", example = "\"localhost\"")
52+
names = "--distributor-host",
53+
description = "Host on which the distributor is listening.")
54+
@ConfigValue(section = DISTRIBUTOR_SECTION, name = "hostname", example = "\"localhost\"")
5055
private String distributorServerHost;
5156

5257
@Parameter(
5358
names = {"--distributor-implementation"},
5459
description = "Full classname of non-default distributor implementation")
5560
@ConfigValue(
56-
section = "distributor",
61+
section = DISTRIBUTOR_SECTION,
5762
name = "implementation",
58-
example = "org.openqa.selenium.grid.distributor.local.LocalDistributor")
59-
private String implementation;
63+
example = DEFAULT_DISTRIBUTOR_IMPLEMENTATION)
64+
private String implementation = DEFAULT_DISTRIBUTOR_IMPLEMENTATION;
6065

6166

6267
@Override

‎java/server/src/org/openqa/selenium/grid/distributor/config/DistributorOptions.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@
2727

2828
public class DistributorOptions {
2929

30-
private static final String DISTRIBUTOR_SECTION = "distributor";
31-
private static final String DEFAULT_DISTRIBUTOR_IMPLEMENTATION =
30+
static final String DISTRIBUTOR_SECTION = "distributor";
31+
static final String DEFAULT_DISTRIBUTOR_IMPLEMENTATION =
3232
"org.openqa.selenium.grid.distributor.local.LocalDistributor";
3333

3434
private final Config config;

‎java/server/src/org/openqa/selenium/grid/log/LoggingFlags.java‎

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,48 +17,62 @@
1717

1818
package org.openqa.selenium.grid.log;
1919

20-
import com.beust.jcommander.Parameter;
20+
import static org.openqa.selenium.grid.config.StandardGridRoles.ALL_ROLES;
21+
import static org.openqa.selenium.grid.log.LoggingOptions.DEFAULT_CONFIGURE_LOGGING;
22+
import static org.openqa.selenium.grid.log.LoggingOptions.DEFAULT_LOG_LEVEL;
23+
import static org.openqa.selenium.grid.log.LoggingOptions.DEFAULT_PLAIN_LOGS;
24+
import static org.openqa.selenium.grid.log.LoggingOptions.DEFAULT_STRUCTURED_LOGS;
25+
import static org.openqa.selenium.grid.log.LoggingOptions.DEFAULT_TRACING_ENABLED;
26+
import static org.openqa.selenium.grid.log.LoggingOptions.LOGGING_SECTION;
27+
2128
import com.google.auto.service.AutoService;
29+
30+
import com.beust.jcommander.Parameter;
31+
2232
import org.openqa.selenium.grid.config.ConfigValue;
2333
import org.openqa.selenium.grid.config.HasRoles;
2434
import org.openqa.selenium.grid.config.Role;
2535

2636
import java.util.Set;
2737

28-
import static org.openqa.selenium.grid.config.StandardGridRoles.ALL_ROLES;
29-
38+
@SuppressWarnings("FieldMayBeFinal")
3039
@AutoService(HasRoles.class)
3140
public class LoggingFlags implements HasRoles {
3241

33-
@Parameter(description = "Configure logging", hidden = true, names = "--configure-logging", arity = 1)
34-
@ConfigValue(section = "logging", name = "enable", example = "true")
35-
private Boolean configureLogging;
42+
@Parameter(
43+
description = "Configure logging",
44+
hidden = true,
45+
names = "--configure-logging",
46+
arity = 1)
47+
@ConfigValue(section = LOGGING_SECTION, name = "enable", example = "true")
48+
private Boolean configureLogging = DEFAULT_CONFIGURE_LOGGING;
3649

3750
@Parameter(description = "Use structured logs", names = "--structured-logs", arity = 1)
38-
@ConfigValue(section = "logging", name = "structured-logs", example = "false")
39-
private Boolean structuredLogs;
51+
@ConfigValue(section = LOGGING_SECTION, name = "structured-logs", example = "false")
52+
private Boolean structuredLogs = DEFAULT_STRUCTURED_LOGS;
4053

4154
@Parameter(description = "Use plain log lines", names = "--plain-logs", arity = 1)
42-
@ConfigValue(section = "logging", name = "plain-logs", example = "true")
43-
private Boolean plainLogs;
55+
@ConfigValue(section = LOGGING_SECTION, name = "plain-logs", example = "true")
56+
private Boolean plainLogs = DEFAULT_PLAIN_LOGS;
4457

4558
@Parameter(description = "Enable trace collection", hidden = true, names = "--tracing", arity = 1)
46-
@ConfigValue(section = "logging", name = "tracing", example = "true")
47-
private Boolean enableTracing;
59+
@ConfigValue(section = LOGGING_SECTION, name = "tracing", example = "true")
60+
private Boolean enableTracing = DEFAULT_TRACING_ENABLED;
4861

4962
@Parameter(description = "File to write out logs", hidden = true, names = "--log", arity = 1)
50-
@ConfigValue(section = "logging", name = "log-file", example = "true")
63+
@ConfigValue(section = LOGGING_SECTION, name = "log-file", example = "true")
5164
private String logFile;
5265

5366
@Parameter(description = "Log encoding", names = "--log-encoding", arity = 1)
54-
@ConfigValue(section = "logging", name = "log-encoding", example = "UTF-8")
67+
@ConfigValue(section = LOGGING_SECTION, name = "log-encoding", example = "UTF-8")
5568
private String logEncoding;
5669

57-
@Parameter(description = "Log level. Default logging level is INFO. Log levels are described here " +
58-
"https://docs.oracle.com/javase/7/docs/api/java/util/logging/Level.html",
70+
@Parameter(description =
71+
"Log level. Default logging level is INFO. Log levels are described here " +
72+
"https://docs.oracle.com/javase/7/docs/api/java/util/logging/Level.html",
5973
names = "--log-level", arity = 1)
60-
@ConfigValue(section = "logging", name = "log-level", example = "INFO")
61-
private String logLevel;
74+
@ConfigValue(section = LOGGING_SECTION, name = "log-level", example = "INFO")
75+
private String logLevel = DEFAULT_LOG_LEVEL;
6276

6377
@Override
6478
public Set<Role> getRoles() {

‎java/server/src/org/openqa/selenium/grid/log/LoggingOptions.java‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,13 @@
3939

4040
public class LoggingOptions {
4141

42+
static final String LOGGING_SECTION = "logging";
43+
static final boolean DEFAULT_CONFIGURE_LOGGING = true;
44+
static final String DEFAULT_LOG_LEVEL = Level.INFO.getName();
45+
static final boolean DEFAULT_PLAIN_LOGS = false;
46+
static final boolean DEFAULT_STRUCTURED_LOGS = false;
47+
static final boolean DEFAULT_TRACING_ENABLED = true;
4248
private static final Logger LOG = Logger.getLogger(LoggingOptions.class.getName());
43-
private static final String LOGGING_SECTION = "logging";
4449
private final Config config;
4550
private Level level = Level.INFO;
4651

@@ -49,19 +54,19 @@ public LoggingOptions(Config config) {
4954
}
5055

5156
public boolean isUsingStructuredLogging() {
52-
return config.getBool(LOGGING_SECTION, "structured-logs").orElse(false);
57+
return config.getBool(LOGGING_SECTION, "structured-logs").orElse(DEFAULT_STRUCTURED_LOGS);
5358
}
5459

5560
public boolean isUsingPlainLogs() {
56-
return config.getBool(LOGGING_SECTION, "plain-logs").orElse(true);
61+
return config.getBool(LOGGING_SECTION, "plain-logs").orElse(DEFAULT_PLAIN_LOGS);
5762
}
5863

5964
public String getLogEncoding() {
6065
return config.get(LOGGING_SECTION, "log-encoding").orElse(null);
6166
}
6267

6368
public void setLoggingLevel() {
64-
String configLevel = config.get(LOGGING_SECTION, "log-level").orElse(Level.INFO.getName());
69+
String configLevel = config.get(LOGGING_SECTION, "log-level").orElse(DEFAULT_LOG_LEVEL);
6570

6671
try {
6772
level = Level.parse(configLevel.toUpperCase(Locale.ROOT));
@@ -71,7 +76,8 @@ public void setLoggingLevel() {
7176
}
7277

7378
public Tracer getTracer() {
74-
boolean tracingEnabled = config.getBool(LOGGING_SECTION, "tracing").orElse(true);
79+
boolean tracingEnabled = config.getBool(LOGGING_SECTION, "tracing")
80+
.orElse(DEFAULT_TRACING_ENABLED);
7581
if (!tracingEnabled) {
7682
LOG.info("Using null tracer");
7783
return new NullTracer();
@@ -81,7 +87,7 @@ public Tracer getTracer() {
8187
}
8288

8389
public void configureLogging() {
84-
if (!config.getBool(LOGGING_SECTION, "enable").orElse(true)) {
90+
if (!config.getBool(LOGGING_SECTION, "enable").orElse(DEFAULT_CONFIGURE_LOGGING)) {
8591
return;
8692
}
8793

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,14 @@ java_library(
99
"//java/server/test/org/openqa/selenium/grid/node/config:__subpackages__",
1010
],
1111
deps = [
12+
"//java:auto-service",
1213
"//java/client/src/org/openqa/selenium/chromium",
1314
"//java/client/src/org/openqa/selenium/json",
1415
"//java/client/src/org/openqa/selenium/remote",
1516
"//java/server/src/org/openqa/selenium/grid/config",
1617
"//java/server/src/org/openqa/selenium/grid/data",
1718
"//java/server/src/org/openqa/selenium/grid/node",
19+
artifact("com.beust:jcommander"),
1820
artifact("com.google.guava:guava"),
1921
],
2022
)

java/server/src/org/openqa/selenium/grid/node/httpd/NodeFlags.java renamed to java/server/src/org/openqa/selenium/grid/node/config/NodeFlags.java

Lines changed: 31 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,15 @@
1515
// specific language governing permissions and limitations
1616
// under the License.
1717

18-
package org.openqa.selenium.grid.node.httpd;
18+
package org.openqa.selenium.grid.node.config;
1919

2020
import static org.openqa.selenium.grid.config.StandardGridRoles.NODE_ROLE;
21+
import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_DETECT_DRIVERS;
22+
import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_HEARTBEAT_PERIOD;
23+
import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_MAX_SESSIONS;
24+
import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_REGISTER_CYCLE;
25+
import static org.openqa.selenium.grid.node.config.NodeOptions.DEFAULT_REGISTER_PERIOD;
26+
import static org.openqa.selenium.grid.node.config.NodeOptions.NODE_SECTION;
2127

2228
import com.google.auto.service.AutoService;
2329

@@ -28,7 +34,6 @@
2834
import org.openqa.selenium.grid.config.NonSplittingSplitter;
2935
import org.openqa.selenium.grid.config.Role;
3036

31-
import java.net.URL;
3237
import java.util.Collections;
3338
import java.util.HashSet;
3439
import java.util.List;
@@ -39,23 +44,26 @@
3944
public class NodeFlags implements HasRoles {
4045

4146
@Parameter(
42-
names = "--max-sessions",
47+
names = "--max-sessions. Default value is the number of available processors.",
4348
description = "Maximum number of concurrent sessions.")
44-
@ConfigValue(section = "node", name = "max-concurrent-sessions", example = "8")
45-
public int maxSessions;
49+
@ConfigValue(section = NODE_SECTION, name = "max-sessions", example = "8")
50+
public int maxSessions = DEFAULT_MAX_SESSIONS;
4651

4752
@Parameter(
4853
names = {"--detect-drivers"}, arity = 1,
4954
description = "Autodetect which drivers are available on the current system, " +
50-
"and add them to the Node. Defaults to true.")
51-
@ConfigValue(section = "node", name = "detect-drivers", example = "true")
52-
public Boolean autoconfigure;
55+
"and add them to the Node.")
56+
@ConfigValue(section = NODE_SECTION, name = "detect-drivers", example = "true")
57+
public Boolean autoconfigure = DEFAULT_DETECT_DRIVERS;
5358

5459
@Parameter(
5560
names = {"-I", "--driver-implementation"},
5661
description = "Drivers that should be checked. If specified, will skip autoconfiguration. " +
5762
"Example: -I \"firefox\" -I \"chrome\"")
58-
@ConfigValue(section = "node", name = "drivers", example = "[\"firefox\", \"chrome\"]")
63+
@ConfigValue(
64+
section = NODE_SECTION,
65+
name = "driver-implementation",
66+
example = "[\"firefox\", \"chrome\"]")
5967
public Set<String> driverNames = new HashSet<>();
6068

6169
@Parameter(
@@ -68,23 +76,23 @@ public class NodeFlags implements HasRoles {
6876
variableArity = true,
6977
splitter = NonSplittingSplitter.class)
7078
@ConfigValue(
71-
section = "node",
79+
section = NODE_SECTION,
7280
name = "driver-factories",
7381
example = "[\"org.openqa.selenium.example.LynxDriverFactory '{\"browserName\": \"lynx\"}']")
7482
public List<String> driverFactory2Config;
7583

7684
@Parameter(
77-
names = {"--public-url"},
85+
names = {"--grid-url"},
7886
description = "Public URL of the Grid as a whole (typically the address of the Hub " +
7987
"or the Router)")
80-
@ConfigValue(section = "node", name = "grid-url", example = "\"https://grid.example.com\"")
81-
public URL gridUri;
88+
@ConfigValue(section = NODE_SECTION, name = "grid-url", example = "\"https://grid.example.com\"")
89+
public String gridUri;
8290

8391
@Parameter(
8492
names = {"--driver-configuration"},
8593
description = "List of configured drivers a Node supports. " +
8694
"It is recommended to provide this type of configuration through a toml config " +
87-
"file to improve readability." +
95+
"file to improve readability. Command line example: " +
8896
"--drivers-configuration name=\"Firefox Nightly\" max-sessions=2 " +
8997
"stereotype='{\"browserName\": \"firefox\", \"browserVersion\": \"86\", " +
9098
"\"moz:firefoxOptions\": " +
@@ -93,7 +101,7 @@ public class NodeFlags implements HasRoles {
93101
variableArity = true,
94102
splitter = NonSplittingSplitter.class)
95103
@ConfigValue(
96-
section = "node",
104+
section = NODE_SECTION,
97105
name = "driver-configuration",
98106
prefixed = true,
99107
example = "\n" +
@@ -106,24 +114,25 @@ public class NodeFlags implements HasRoles {
106114

107115
@Parameter(
108116
names = "--register-cycle",
109-
description = "How often, in seconds, the Node will try to register itself for the first time to the Distributor.")
110-
@ConfigValue(section = "node", name = "register-cycle", example = "10")
111-
public int registerCycle;
117+
description = "How often, in seconds, the Node will try to register itself for "
118+
+ "the first time to the Distributor.")
119+
@ConfigValue(section = NODE_SECTION, name = "register-cycle", example = "10")
120+
public int registerCycle = DEFAULT_REGISTER_CYCLE;
112121

113122
@Parameter(
114123
names = "--register-period",
115124
description = "How long, in seconds, will the Node try to register to the Distributor for " +
116125
"the first time. After this period is completed, the Node will not attempt " +
117126
"to register again.")
118-
@ConfigValue(section = "node", name = "register-period", example = "120")
119-
public int registerPeriod;
127+
@ConfigValue(section = NODE_SECTION, name = "register-period", example = "120")
128+
public int registerPeriod = DEFAULT_REGISTER_PERIOD;
120129

121130
@Parameter(
122131
names = "--heartbeat-period",
123132
description = "How often, in seconds, will the Node send heartbeat events to the Distributor " +
124133
"to inform it that the Node is up.")
125-
@ConfigValue(section = "node", name = "heartbeat-period", example = "10")
126-
public int heartbeatPeriod;
134+
@ConfigValue(section = NODE_SECTION, name = "heartbeat-period", example = "10")
135+
public int heartbeatPeriod = DEFAULT_HEARTBEAT_PERIOD;
127136

128137
@Override
129138
public Set<Role> getRoles() {

0 commit comments

Comments
 (0)