github-actions[bot] commented on code in PR #63534:
URL: https://github.com/apache/doris/pull/63534#discussion_r3301837043
##########
docker/runtime/doris-compose/command.py:
##########
@@ -624,13 +653,20 @@ def run(self, args):
if args.cloud:
args.sql_mode_node_mgr = True
+ instance_id = getattr(args, 'instance_id', None)
+ cluster_snapshot = getattr(args, 'cluster_snapshot', '')
+ enable_storage_vault = CLUSTER.is_true(
+ CLUSTER.get_env_value(args.env, "ENABLE_STORAGE_VAULT"))
+
cluster = CLUSTER.Cluster.new(
args.NAME, args.IMAGE, args.cloud, args.root, args.fe_config,
args.be_config, args.ms_config, args.recycle_config,
args.remote_master_fe, args.local_network_ip, args.fe_follower,
args.be_disks, args.be_cluster, args.reg_be, args.extra_hosts,
args.coverage_dir, cloud_store_config, args.sql_mode_node_mgr,
- args.be_metaservice_endpoint, args.be_cluster_id, args.tde_ak,
args.tde_sk)
+ args.be_metaservice_endpoint, args.be_cluster_id, args.tde_ak,
args.tde_sk,
+ external_ms_cluster, instance_id, cluster_snapshot,
+ enable_storage_vault)
Review Comment:
This makes every new `up` invocation fail before the cluster is created:
`external_ms_cluster` is never assigned anywhere in `run`, while the parser
stores the value as `args.external_ms`. Python will raise `NameError` here even
when `--external-ms` is not provided. Please initialize this from
`args.external_ms` before calling `Cluster.new` (and handle the intended
no-external-MS default).
##########
docker/runtime/doris-compose/resource/init_fe.sh:
##########
@@ -197,6 +208,48 @@ start_cloud_fe() {
fi
touch $REGISTER_FILE
+}
+
+start_cloud_fe() {
+ if [ -f "$REGISTER_FILE" ] || [ -n "${CLUSTER_SNAPSHOT_FILE}" ]; then
Review Comment:
Defining `start_cloud_fe` a second time shadows the earlier definition in
this shell script, so the recovery-script handling above
(`restore_snapshot.sh`, `--metadata_failure_recovery`, and
`--recovery_journal_id`) is now dead code. Existing cloud FE recovery startups
will skip that logic and start normally instead. Please merge the new
snapshot/auto-create flow into the original function instead of redefining it.
##########
regression-test/framework/src/main/groovy/org/apache/doris/regression/suite/SuiteCluster.groovy:
##########
@@ -356,6 +376,14 @@ class SuiteCluster {
cmd += ['--extra-hosts']
cmd += options.extraHosts
}
+ def envs = new ArrayList<String>(options.environments)
+ if (options.enableStorageVault) {
Review Comment:
`ClusterOptions` does not define an `environments` property, so
`SuiteCluster.init` will fail as soon as any docker suite initializes a
cluster, before it can even build the compose command. Please add the option
field (for example an empty `List<String> environments = []`) or use an
existing configuration source.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
