Skip to content

[Fix-17177] Fix cos resource can not list #17178

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.

Sign up for GitHub

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

Jump to bottom
Merged
SbloodyS merged 6 commits into from
May 22, 2025
Merged

[Fix-17177] Fix cos resource can not list #17178

SbloodyS merged 6 commits into from
May 22, 2025

Conversation

@gaoyan1998
Copy link
Contributor

@gaoyan1998 gaoyan1998 commented May 13, 2025 •
edited
Loading

Purpose of the pull request

close #17177

Brief change log

fix cos can not list
fix cos list lost folders

@gaoyan1998 gaoyan1998 mentioned this pull request May 13, 2025
Closed
3 tasks
@ruanwenjun
Copy link
Member

ruanwenjun commented May 14, 2025

Please follow the code style of S3StorageOperator, it seems this is caused by the directory doesn't end with '/'?

@gaoyan1998
Copy link
Contributor Author

gaoyan1998 commented May 14, 2025 •
edited
Loading

Please follow the code style of S3StorageOperator, it seems this is caused by the directory doesn't end with '/'?

@ruanwenjun hi
I'm sorry I couldn't see where the problem was. Looking at the error, it was a Python E2E test problem, but from the logs, it seems that it was not caused by my pr code. It seems that the installation of the package in testcontainer failed? I don't know if I understand it right

2025-05-13 14:11:01,088 tc.docker 35 [Thread-3] INFO  [] - #5 ERROR: process "/bin/sh -c apt update     && apt install -y software-properties-common     && add-apt-repository ppa:deadsnakes/ppa     && apt update     && apt-get install -y python3.8 libpython3.8-dev python3.8-dev python3.8-distutils     && rm -rf /var/lib/apt/lists/*" did not complete successfully: exit code: 100
2025-05-13 14:11:01,088 tc.docker 35 [Thread-3] INFO  [] - ------
2025-05-13 14:11:01,088 tc.docker 35 [Thread-3] INFO  [] -  > [dolphinscheduler 2/2] RUN apt update     && apt install -y software-properties-common     && add-apt-repository ppa:deadsnakes/ppa     && apt update     && apt-get install -y python3.8 libpython3.8-dev python3.8-dev python3.8-distutils     && rm -rf /var/lib/apt/lists/*:
2025-05-13 14:11:01,088 tc.docker 35 [Thread-3] INFO  [] - 61.80 E: Failed to fetch http://archive.ubuntu.com/ubuntu/dists/noble-updates/main/dep11/Components-amd64.yml.gz  File has unexpected size (216476 != 215904). Mirror sync in progress? [IP: 91.189.91.83 80]
2025-05-13 14:11:01,088 tc.docker 35 [Thread-3] INFO  [] - 61.80    Hashes of expected file:
2025-05-13 14:11:01,088 tc.docker 35 [Thread-3] INFO  [] - 61.80     - Filesize:215904 [weak]
2025-05-13 14:11:01,088 tc.docker 35 [Thread-3] INFO  [] - 61.80     - SHA256:2237a806c02faf36f588e754007941fd5504ac77446236b11f6b3497d6c4bcbd
2025-05-13 14:11:01,088 tc.docker 35 [Thread-3] INFO  [] - 61.80     - SHA1:eb33fce73db3b526008cefb35e92fd854bfec4d3 [weak]
2025-05-13 14:11:01,088 tc.docker 35 [Thread-3] INFO  [] - 61.80     - MD5Sum:4c687407a448453219209b3be60411a2 [weak]
2025-05-13 14:11:01,088 tc.docker 35 [Thread-3] INFO  [] - 61.80    Release file created at: Tue, 13 May 2025 12:44:45 +0000
2025-05-13 14:11:01,088 tc.docker 35 [Thread-3] INFO  [] - 61.80 E: Failed to fetch http://archive.ubuntu.com/ubuntu/dists/noble-updates/universe/dep11/Components-amd64.yml.gz  
2025-05-13 14:11:01,088 tc.docker 35 [Thread-3] INFO  [] - 61.80 E: Failed to fetch http://archive.ubuntu.com/ubuntu/dists/noble-updates/restricted/dep11/Components-amd64.yml.gz  
2025-05-13 14:11:01,088 tc.docker 35 [Thread-3] INFO  [] - 61.80 E: Some index files failed to download. They have been ignored, or old ones used instead.

@ruanwenjun ruanwenjun force-pushed the fix-cos-bug-17177 branch from 54ead0a to c7ccace Compare May 14, 2025 15:35
@ruanwenjun
Copy link
Member

ruanwenjun commented May 14, 2025

Please follow the code style of S3StorageOperator, it seems this is caused by the directory doesn't end with '/'?

If the resourceAbsolutePath is an object, can this work?

@gaoyan1998
Copy link
Contributor Author

gaoyan1998 commented May 15, 2025

Please follow the code style of S3StorageOperator, it seems this is caused by the directory doesn't end with '/'?

If the resourceAbsolutePath is an object, can this work?

The function listStorageEntity receive a String type Won't pass in the Object type, Under what circumstances will it become an Object, and what data is this Object?

@ruanwenjun
Copy link
Member

ruanwenjun commented May 15, 2025

Please follow the code style of S3StorageOperator, it seems this is caused by the directory doesn't end with '/'?

If the resourceAbsolutePath is an object, can this work?

The function listStorageEntity receive a String type Won't pass in the Object type, Under what circumstances will it become an Object, and what data is this Object?

Sorry, I mean if the path is a file not directory, can this work?

@gaoyan1998
Copy link
Contributor Author

gaoyan1998 commented May 15, 2025 •
edited
Loading

Please follow the code style of S3StorageOperator, it seems this is caused by the directory doesn't end with '/'?

If the resourceAbsolutePath is an object, can this work?

The function listStorageEntity receive a String type Won't pass in the Object type, Under what circumstances will it become an Object, and what data is this Object?

Sorry, I mean if the path is a file not directory, can this work?

not work, But this method is to list the details of the directory, when will pass a file path?

@ruanwenjun
Copy link
Member

ruanwenjun commented May 15, 2025

Please follow the code style of S3StorageOperator, it seems this is caused by the directory doesn't end with '/'?

If the resourceAbsolutePath is an object, can this work?

The function listStorageEntity receive a String type Won't pass in the Object type, Under what circumstances will it become an Object, and what data is this Object?

Sorry, I mean if the path is a file not directory, can this work?

not work, But this method is to list the details of the directory, when will pass a file path?

You need to follow the interface definition

dolphinscheduler/dolphinscheduler-storage-plugin/dolphinscheduler-storage-api/src/main/java/org/apache/dolphinscheduler/plugin/storage/api/StorageOperator.java

Lines 132 to 140 in bdec8b1

/**
* Return the {@link StorageEntity} under the given path.
* <p>If the path is a file, return the file status.
* <p>If the path is a directory, return the file/directory under the directory.
* <p>If the path is not exist, will return empty.
*
* @param resourceAbsolutePath the resource absolute path, cannot be empty
*/
List<StorageEntity> listStorageEntity(String resourceAbsolutePath);

I have point this before #17178 (comment)

gaoyan1998 added 3 commits May 16, 2025 10:38
@gaoyan1998
fix(storage): fix file and directory query logic in COS storage plugin**
a27c986 
- Optimize the listStorageEntity method to handle queries for files and directories
- Add a check for whether the path ends with `File.separator` to distinguish between file and directory queries
- Add object existence checks to improve query efficiency and accuracy
@gaoyan1998
fix(storage): fix file and directory query logic in COS storage plugin**
6304f92 
- Optimize the listStorageEntity method to handle queries for files and directories
- Add a check for whether the path ends with `File.separator` to distinguish between file and directory queries
- Add object existence checks to improve query efficiency and accuracy
@gaoyan1998
formate code
edefe67
@gaoyan1998
Copy link
Contributor Author

gaoyan1998 commented May 16, 2025

@ruanwenjun PTAL
I've revised the code logic as follows:

Directory Intent: If the input path ends with File.separator, it is directly treated as a directory for querying.
Ambiguous Paths: If the path does not end with File.separator, the system first checks if it matches an existing file. If found, the file is returned; otherwise, it falls back to querying as a directory.

If the path is a file not directory, it`s still work well

Known Issues:

Naming Conflict: If a directory contains both a file and a folder with the same name (and the input path lacks a trailing File.separator), the file will be returned by default. This is intentional—users should append File.separator to explicitly target directories.

Extension Check: The current method to distinguish files from directories (checking for a . in the path) is not entirely reliable. This is a separate issue and will be addressed in a future PR if needed.

@sonarqubecloud
Copy link

sonarqubecloud bot commented May 19, 2025

Please retry analysis of this Pull-Request directly on SonarQube Cloud

ruanwenjun
ruanwenjun approved these changes May 22, 2025
View reviewed changes
Copy link
Member

@ruanwenjun ruanwenjun left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM.

Right now, we can only judge whether the path is file or directory by file name, this is incorrect when the file doesn't have extention, since we don't store the file metadata, we need to provide an additional parameter isDirectory in the controller, then listStorageEntity can only used to visit the directory.

SbloodyS
SbloodyS approved these changes May 22, 2025
View reviewed changes
Copy link
Member

@SbloodyS SbloodyS left a comment

Choose a reason for hiding this comment

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

+1

@SbloodyS SbloodyS added the bug Something isn't working label May 22, 2025
@SbloodyS SbloodyS added this to the 3.3.1 milestone May 22, 2025
@SbloodyS SbloodyS merged commit a5296d6 into apache:dev May 22, 2025
70 of 72 checks passed
@codeOwlAI codeOwlAI mentioned this pull request Jun 30, 2025
Open
davidzollo pushed a commit to davidzollo/dolphinscheduler that referenced this pull request Oct 27, 2025
@gaoyan1998 @davidzollo
[Fix-17177] Fix cos resource can not list (apache#17178)
58241aa
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SbloodyS SbloodyS SbloodyS approved these changes

@ruanwenjun ruanwenjun ruanwenjun approved these changes

Assignees

@gaoyan1998 gaoyan1998

Labels

backend bug Something isn't working

Projects

None yet

Milestone

3.3.1

Development

Successfully merging this pull request may close these issues.

[Bug] [API] COS Resource can not show list

3 participants

@gaoyan1998 @ruanwenjun @SbloodyS