[Improve] support delete flink home #2582
Conversation
zhoulii
force-pushed
the
support-remove-flink-home
branch
from
9406a74 to
66572eb
Compare
There was a problem hiding this comment.
@zhoulii Thanks for your contribution & quick-drive
I left a few of comments. PTAL in your free time.
May the items from my limit read as follows get your @wolfboys @zhoulii confirmation ?
- If there are running jobs or sessions using the target flinkhome, it would be better not to support changing Flinkhome to avoid abnormal risks when some savepoints are triggered.
- The
delete,update,createapi have noApiAccessorXXXPermissionannotations . Is here any risk ?
I'd appreciated with your confirmation.
thanks~
| @@ -39,6 +42,8 @@ | |||
| public class FlinkEnvServiceImpl extends ServiceImpl<FlinkEnvMapper, FlinkEnv> | |||
| implements FlinkEnvService { | |||
|
|
|||
| @Autowired private FlinkClusterService flinkClusterService; | |||
| @Autowired private ApplicationService applicationService; | |||
There was a problem hiding this comment.
| @Autowired private ApplicationService applicationService; | |
| @Autowired private ApplicationService applicationService; | |
| LambdaQueryWrapper<Application> lambdaQueryWrapper = | ||
| new LambdaQueryWrapper<Application>().eq(Application::getVersionId, flinkEnvId); | ||
| return this.count(lambdaQueryWrapper) > 0; |
There was a problem hiding this comment.
May here use getBaseMapper() .exists(.... ?
| LambdaQueryWrapper<FlinkCluster> lambdaQueryWrapper = | ||
| new LambdaQueryWrapper<FlinkCluster>().eq(FlinkCluster::getVersionId, flinkEnvId); | ||
| return this.count(lambdaQueryWrapper) > 0; |
There was a problem hiding this comment.
same as mentioned above.
| @@ -18,6 +18,8 @@ export default { | |||
| title: 'Flink Home', | |||
| conf: 'Flink 配置', | |||
| sync: '配置同步', | |||
| edit: '编辑 Flink Home', | |||
| delete: '确定要删除此 Flink home ?', | |||
There was a problem hiding this comment.
I checked the header of the edit tab window is :
There was a problem hiding this comment.
This is due to Adding and Editing Flink Home use the same modal#L169, and the title is fixed. I have limited understanding of frontend, what I can do is change the title to Add/Edit. but maybe it's not a elegant way.
There was a problem hiding this comment.
@zhoulii Take it easy, please ~ 😁
Hi, @wangsizhu0504 Could you help take a look about this ?
Thanks a lot~
@RocMarshal Thanks for the review. I agree with you, maybe we can hanle these in another pr, and let this one focus on delete operation. what do you think? |
|
I agree with you, We can do it in next pr. |
| await fetchFlinkEnvRemove(item.id); | ||
| await getFlinkSetting(); | ||
| createMessage.success('The current flink home is removed'); | ||
| } |
There was a problem hiding this comment.
async function handleDelete(item: FlinkEnv) {
const resp = await fetchFlinkEnvRemove(item.id);
if (resp.data.code == 200) {
await getFlinkSetting();
createMessage.success('The current flink home is removed');
}
}
What changes were proposed in this pull request
Issue Number: close #2542
Brief change log
Verifying this change
Does this pull request potentially affect one of the following parts