ruanwenjun commented on code in PR #18349:
URL:
https://github.com/apache/dolphinscheduler/pull/18349#discussion_r3414211818
##########
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:
1. Can `workerGroupDao.queryWorkerGroupByName` return list?
2. If the worker group is not exist, need to throw exception rather than
return null.
3. We need to use batch query, rather than query single item at a for-each.
##########
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:
The worker group name shouldn't be deprecated.
##########
dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_mysql.sql:
##########
@@ -1149,7 +1149,8 @@ DROP TABLE IF EXISTS
`t_ds_environment_worker_group_relation`;
CREATE TABLE `t_ds_environment_worker_group_relation` (
`id` bigint(11) NOT NULL AUTO_INCREMENT COMMENT 'id',
`environment_code` bigint(20) NOT NULL COMMENT 'environment code',
- `worker_group` varchar(255) NOT NULL COMMENT 'worker group id',
+ `worker_group_id` int(11) DEFAULT NULL COMMENT 'worker group id',
+ `worker_group` varchar(255) NOT NULL COMMENT 'worker group name',
Review Comment:
Why `worker_group` is not null but `worker_group_id` can be null?
--
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]
