qiuyanjun888 commented on code in PR #18349:
URL:
https://github.com/apache/dolphinscheduler/pull/18349#discussion_r3432944114
##########
dolphinscheduler-dao/src/main/resources/sql/upgrade/3.4.1_schema/mysql/dolphinscheduler_ddl.sql:
##########
@@ -16,4 +16,7 @@
*/
ALTER TABLE `t_ds_serial_command`
-MODIFY COLUMN `workflow_definition_code` BIGINT(20) NOT NULL COMMENT 'workflow
definition code';
\ No newline at end of file
+MODIFY COLUMN `workflow_definition_code` BIGINT(20) NOT NULL COMMENT 'workflow
definition code';
+
+ALTER TABLE `t_ds_environment_worker_group_relation`
+ADD COLUMN `worker_group_id` int(11) DEFAULT NULL COMMENT 'worker group id';
Review Comment:
Addressed on the current branch: the MySQL worker_group_id upgrade DDL/DML
is under sql/upgrade/3.5.0_schema, and the 3.4.1 MySQL DDL no longer contains
this change.
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/EnvironmentServiceImpl.java:
##########
@@ -418,6 +425,14 @@ private void
checkUsedEnvironmentWorkerGroupRelation(Set<String> deleteKeySet,
}
}
+ private Integer getWorkerGroupId(String workerGroupName) {
+ List<WorkerGroup> workerGroups =
workerGroupDao.queryWorkerGroupByName(workerGroupName);
+ if (CollectionUtils.isEmpty(workerGroups)) {
+ return null;
+ }
+ return workerGroups.get(0).getId();
+ }
+
Review Comment:
Addressed on the current branch: WorkerGroupDao/Mapper returns a list,
EnvironmentServiceImpl performs a single queryWorkerGroupByNames batch lookup,
maps results by name, and throws WORKER_GROUP_NOT_EXIST when any requested
worker group is missing.
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/WorkerGroupServiceImpl.java:
##########
@@ -182,7 +182,9 @@ private void checkWorkerGroupDependencies(WorkerGroup
workerGroup) {
// check if the worker group has any dependent environments
List<EnvironmentWorkerGroupRelation> environmentWorkerGroupRelations =
environmentWorkerGroupRelationMapper.selectList(new
QueryWrapper<EnvironmentWorkerGroupRelation>()
-
.lambda().eq(EnvironmentWorkerGroupRelation::getWorkerGroup,
workerGroup.getName()));
+ .eq("worker_group_id", workerGroup.getId())
+ .or(queryWrapper ->
queryWrapper.isNull("worker_group_id")
+ .eq("worker_group", workerGroup.getName())));
Review Comment:
Addressed on the current branch: workerGroup remains a non-deprecated
compatibility field, and worker group deletion dependency checks match
environment relations by worker_group_id or by worker_group name.
##########
dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/WorkerGroupMapper.xml:
##########
@@ -28,6 +28,14 @@
from t_ds_worker_group
where name = #{name}
</select>
+ <select id="queryWorkerGroupByNames"
resultType="org.apache.dolphinscheduler.dao.entity.WorkerGroup">
+ select *
Review Comment:
Addressed on the current branch: queryWorkerGroupByNames uses an explicit
column list instead of a wildcard select.
##########
dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/entity/EnvironmentWorkerGroupRelation.java:
##########
@@ -37,6 +37,11 @@ public class EnvironmentWorkerGroupRelation {
/**
* worker group id
*/
+ private Integer workerGroupId;
+
+ /**
+ * worker group name
+ */
Review Comment:
Addressed on the current branch: EnvironmentWorkerGroupRelation keeps
workerGroup without a deprecated marker in that area.
--
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]
