Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] [Api] Resource files need to be checked before submitting #13887

Closed
wants to merge 1 commit into from

Conversation

qingwli
Copy link
Member

@qingwli qingwli commented Apr 7, 2023

@SbloodyS SbloodyS added 3.2.0 for 3.2.0 version bug Something isn't working labels Apr 7, 2023
@SbloodyS SbloodyS added this to the 3.2.0 milestone Apr 7, 2023
Arrays.stream(this.resourceIds.split(",")).map(Integer::parseInt).toArray(Integer[]::new);

if (resourceIdArray.length > 0) {
List<Resource> list = processService.listResourceByIds(resourceIdArray);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we have refactored the resource center in #12076, ResourceMapper has been deprecated. I think we should not do this way.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we have refactored the resource center in #12076, ResourceMapper has been deprecated. I think we should not do this way.

Add this pre-check and throw an explicit error msg during task execution, which one is better?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add this pre-check and throw an explicit error msg during task execution, which one is better?

If the user deletes files in third-party storage after passing the pre check, it also cannot avoid runtime errors...

Copy link
Member Author

@qingwli qingwli Apr 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In our scenario, we are currently using the 3.1.1 version and I don't know we have refactored about resource center in dev.

And for the 3.1.1 version, we encountered this situation.

Here are the prerequisites:

  1. User can't access third-party storage directly.
  2. User only can operate third-party by ds platform.
  3. We can't delete the resource that binds online released jobs.

process -> task job -> resource table -> third-party storage

And if a process is maintained by a team. One member A operates offline this process. Another member B can delete resource binds in this process by ds. Member A re-online this process and got successful. Will throw an exception during runtime. And If we add a pre-check will find this exception when online.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I agree if the user deletes a third-party storage file directly and we can do nothing about this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code only takes effect in version 3.1.X. And I'm +0 on this. cc @ruanwenjun @zhongjiajie @EricGao888 @caishunfeng

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can post this change to 3.1.6, not dev. Because dev is totally different.

@@ -293,6 +293,7 @@ public enum Status {
RESOURCE_HAS_FOLDER(20018, "There are files or folders in the current directory:{0}", "当前目录下有文件或文件夹[{0}]"),

REMOVE_TASK_INSTANCE_CACHE_ERROR(20019, "remove task instance cache error", "删除任务实例缓存错误"),
TASK_RESOURCE_NOT_EXIST(20020, "Task {0} contains removed resource", "任务[{0}]含有被删除的资源文件"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's better to return the resource doesn't exist

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity for 120 days. It will be closed in 7 days if no further activity occurs.

@github-actions github-actions bot added the Stale label Aug 11, 2023
@github-actions
Copy link

This pull request has been closed because it has not had recent activity. You could reopen it if you try to continue your work, and anyone who are interested in it are encouraged to continue work on this pull request.

@github-actions github-actions bot closed this Aug 18, 2023
@qingwli qingwli deleted the resource-check branch September 7, 2023 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.2.0 for 3.2.0 version backend bug Something isn't working Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] [Api] Run process will throw exception when resource removed
4 participants