-
Notifications
You must be signed in to change notification settings - Fork 500
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
fix(services/dropbox): fix dropbox ci #4501
base: main
Are you sure you want to change the base?
Conversation
@@ -558,6 +563,11 @@ pub async fn test_list_dir_with_recursive_no_trailing_slash(op: Operator) -> Res | |||
} | |||
|
|||
pub async fn test_list_file_with_recursive(op: Operator) -> Result<()> { | |||
// Dropbox does not support list file with recursive. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't make sense. All services that implement list_dir should have list prefix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about this, I don't see it described in the API documentation, and in my practice Dropbox doesn't implement prefix matching.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prefix matching is simulated at
opendal/core/src/layers/complete.rs
Lines 276 to 322 in 755de58
async fn complete_list( | |
&self, | |
path: &str, | |
args: OpList, | |
) -> Result<(RpList, CompleteLister<A, A::Lister>)> { | |
let cap = self.meta.full_capability(); | |
if !cap.list { | |
return Err(self.new_unsupported_error(Operation::List)); | |
} | |
let recursive = args.recursive(); | |
match (recursive, cap.list_with_recursive) { | |
// - If service can list_with_recursive, we can forward list to it directly. | |
(_, true) => { | |
let (rp, p) = self.inner.list(path, args).await?; | |
Ok((rp, CompleteLister::One(p))) | |
} | |
// If recursive is true but service can't list_with_recursive | |
(true, false) => { | |
// Forward path that ends with / | |
if path.ends_with('/') { | |
let p = FlatLister::new(self.inner.clone(), path); | |
Ok((RpList::default(), CompleteLister::Two(p))) | |
} else { | |
let parent = get_parent(path); | |
let p = FlatLister::new(self.inner.clone(), parent); | |
let p = PrefixLister::new(p, path); | |
Ok((RpList::default(), CompleteLister::Four(p))) | |
} | |
} | |
// If recursive and service doesn't support list_with_recursive, we need to handle | |
// list prefix by ourselves. | |
(false, false) => { | |
// Forward path that ends with / | |
if path.ends_with('/') { | |
let (rp, p) = self.inner.list(path, args).await?; | |
Ok((rp, CompleteLister::One(p))) | |
} else { | |
let parent = get_parent(path); | |
let (rp, p) = self.inner.list(parent, args).await?; | |
let p = PrefixLister::new(p, path); | |
Ok((rp, CompleteLister::Three(p))) | |
} | |
} | |
} | |
} |
I have a plan to refactor them later to make those behavior more clear.
core/tests/behavior/async_rename.rs
Outdated
@@ -178,6 +178,11 @@ pub async fn test_rename_nested(op: Operator) -> Result<()> { | |||
|
|||
/// Rename to a exist path should overwrite successfully. | |||
pub async fn test_rename_overwrite(op: Operator) -> Result<()> { | |||
// Dropbox does not support rename overwrite. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can remove existing file before rename/copy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
Is this still relevant? It seems no dropbox CI failures recently. |
dropbox CI has been disabled. |
fix: #4484
There are several issues that need to be fixed here, related ci: https://github.com/apache/opendal/actions/runs/8682043318/job/23805843756.
Error caused by Dropbox service not supporting:
Should return an error when it does not exist:
Returned by list does not contact the complete path:
List dir should only return dir/