sashapolo commented on code in PR #2879:
URL: https://github.com/apache/ignite-3/pull/2879#discussion_r1405978579
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/TestIgnitionManager.java:
##########
@@ -76,20 +94,36 @@ public static CompletableFuture<Ignite> start(String
nodeName, @Nullable String
try {
Files.createDirectories(workDir);
Path configPath = workDir.resolve(DEFAULT_CONFIG_NAME);
- if (configStr == null) {
- if (Files.notExists(configPath)) {
- Files.createFile(configPath);
- }
- } else {
- Files.writeString(configPath, configStr,
- StandardOpenOption.SYNC, StandardOpenOption.CREATE,
StandardOpenOption.TRUNCATE_EXISTING);
- }
+
+ addDefaultsToConfigurationFile(configStr, configPath);
+
return IgnitionManager.start(nodeName, configPath, workDir);
} catch (IOException e) {
throw new IgniteException("Couldn't write node config.", e);
}
}
+ /**
+ * Writes default values into the configuration file, according to the
same rules that are used in {@link #start(String, String, Path)}.
+ */
+ public static void addDefaultsToConfigurationFile(Path configPath) {
+ try {
+ addDefaultsToConfigurationFile(null, configPath);
+ } catch (IOException e) {
+ throw new IgniteException("Couldn't update node configuration
file", e);
+ }
+ }
+
+ private static void addDefaultsToConfigurationFile(@Nullable String
configStr, Path configPath) throws IOException {
+ if (configStr == null && Files.exists(configPath)) {
+ // Nothing to do.
+ return;
+ }
+
+ Files.writeString(configPath, applyTestDefaultsToConfig(configStr,
DEFAULT_NODE_CONFIG),
+ StandardOpenOption.SYNC, StandardOpenOption.CREATE,
StandardOpenOption.TRUNCATE_EXISTING);
Review Comment:
Same here about the static import
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/TestIgnitionManager.java:
##########
@@ -108,33 +142,34 @@ private static InitParameters
applyTestDefaultsToClusterConfig(InitParameters pa
.metaStorageNodeNames(params.metaStorageNodeNames())
.cmgNodeNames(params.cmgNodeNames());
+
builder.clusterConfiguration(applyTestDefaultsToConfig(params.clusterConfiguration(),
DEFAULT_CLUSTER_CONFIG));
+
+ return builder.build();
+ }
+
+ private static String applyTestDefaultsToConfig(@Nullable String
configStr, Map<String, String> defaults) {
+ if (configStr == null) {
+ configStr = "{}";
+ }
+
ConfigDocument configDocument;
- if (params.clusterConfiguration() == null) {
- configDocument = ConfigDocumentFactory.parseString("{}");
- } else {
- configDocument =
ConfigDocumentFactory.parseString(params.clusterConfiguration());
+ try {
+ configDocument = ConfigDocumentFactory.parseString(configStr);
+ } catch (ConfigException e) {
+ // Preserve original broken content, it might be broken on purpose.
Review Comment:
Sorry, what do you mean by "broken on purpose"?
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/TestIgnitionManager.java:
##########
@@ -45,6 +49,20 @@ public class TestIgnitionManager {
/** Default partition idle SafeTime interval in ms used for tests that is
set on node init. */
public static final int DEFAULT_PARTITION_IDLE_SYNC_TIME_INTERVAL_MS = 100;
+ /** Map with default node configuration values. */
+ private static final Map<String, String> DEFAULT_NODE_CONFIG = Map.of(
+ "aipersist.defaultRegion.size", Integer.toString(256 *
Constants.MiB),
Review Comment:
I'd have a static import for `Constants` but that's up to you
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
