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.

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

refactor(core): Deprecate OpList::version and add versions instead #5481

Merged
merged 7 commits into from
Dec 31, 2024
Merged

refactor(core): Deprecate OpList::version and add versions instead #5481

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d67cead
Deprecate version and add versioned
geetanshjuneja Dec 30, 2024
6be969c
deprecate oplist::version and change use of version to versioned
geetanshjuneja Dec 30, 2024
08deaf4
refactor
geetanshjuneja Dec 30, 2024
eb451db
ruby test fix
geetanshjuneja Dec 30, 2024
b6a8ef0
test fix
geetanshjuneja Dec 30, 2024
cd886b5
more fn and fields deprecated
geetanshjuneja Dec 31, 2024
76794d5
Use versions for better reading
Xuanwo Dec 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions core/src/layers/capability_check.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ impl<A: Access> LayeredAccess for CapabilityAccessor<A> {

async fn list(&self, path: &str, args: OpList) -> crate::Result<(RpList, Self::Lister)> {
let capability = self.info.full_capability();
if !capability.list_with_version && args.version() {
if !capability.list_with_version && args.versioned() {
return Err(new_unsupported_error(
self.info.as_ref(),
Operation::List,
Expand Down Expand Up @@ -191,7 +191,7 @@ impl<A: Access> LayeredAccess for CapabilityAccessor<A> {
args: OpList,
) -> crate::Result<(RpList, Self::BlockingLister)> {
let capability = self.info.full_capability();
if !capability.list_with_version && args.version() {
if !capability.list_with_version && args.versioned() {
Copy link
Member

Choose a reason for hiding this comment

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

list_with_version should also be depreated by list_with_versioned.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem I'll add that all also.

return Err(new_unsupported_error(
self.info.as_ref(),
Operation::BlockingList,
Expand Down Expand Up @@ -289,7 +289,7 @@ mod tests {
list: true,
..Default::default()
});
let res = op.list_with("path/").version(true).await;
let res = op.list_with("path/").versioned(true).await;
assert!(res.is_err());
assert_eq!(res.unwrap_err().kind(), ErrorKind::Unsupported);

Expand Down
6 changes: 6 additions & 0 deletions core/src/raw/ops.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,15 @@ impl OpList {
}

/// Get the version of this list operation
#[deprecated = "use versioned instead"]
pub fn version(&self) -> bool {
self.version
}

/// Get the version of this list operation
pub fn versioned(&self) -> bool {
Copy link
Member

Choose a reason for hiding this comment

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

with_version should be replaced by with_versioned.

self.version
Copy link
Member

Choose a reason for hiding this comment

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

Should be changed by self.versioned

}
}

/// Args for `presign` operation.
Expand Down
2 changes: 1 addition & 1 deletion core/src/services/s3/backend.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1060,7 +1060,7 @@ impl Access for S3Backend {
}

async fn list(&self, path: &str, args: OpList) -> Result<(RpList, Self::Lister)> {
let l = if args.version() {
let l = if args.versioned() {
TwoWays::Two(PageLister::new(S3ObjectVersionsLister::new(
self.core.clone(),
path,
Expand Down
12 changes: 12 additions & 0 deletions core/src/types/operator/operator_futures.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -486,9 +486,21 @@ impl<F: Future<Output = Result<Vec<Entry>>>> FutureList<F> {
/// by the underlying service
///
/// Default to `false`
#[deprecated = "use versioned instead"]
Copy link
Member

Choose a reason for hiding this comment

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

Please also add a version field: 0.51.1

pub fn version(self, v: bool) -> Self {
self.map(|args| args.with_version(v))
}

/// The version is used to control whether the object versions should be returned.
///
/// - If `false`, list operation will not return with object versions
/// - If `true`, list operation will return with object versions if object versioning is supported
/// by the underlying service
///
/// Default to `false`
pub fn versioned(self, v: bool) -> Self {
self.map(|args| args.with_version(v))
Copy link
Member

Choose a reason for hiding this comment

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

Please also deprecate all use of OpList::version and change them to OpList::versioned.

}
}

/// Future that generated by [`Operator::list_with`] or [`Operator::lister_with`].
Expand Down
Loading