Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions core/services/github/src/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ impl Builder for GithubBuilder {
write_can_empty: true,

delete: true,
delete_with_if_match: true,

list: true,
list_with_recursive: true,
Expand Down
40 changes: 31 additions & 9 deletions core/services/github/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ impl GithubCore {
self.send(ctx, req).await
}

pub async fn delete(&self, ctx: &OperationContext, path: &str) -> Result<()> {
pub async fn delete(&self, ctx: &OperationContext, path: &str, args: &OpDelete) -> Result<()> {
// If path is a directory, we should delete path/.gitkeep
let formatted_path = format!("{path}.gitkeep");
let p = if path.ends_with('/') {
Expand All @@ -220,8 +220,12 @@ impl GithubCore {
path
};

let Some(sha) = self.get_file_sha(ctx, p).await? else {
return Ok(());
let sha = match args.if_match() {
Some(sha) => sha.to_string(),
None => match self.get_file_sha(ctx, p).await? {
Some(sha) => sha,
None => return Ok(()),
},
};

let path = build_abs_path(&self.root, p);
Expand Down Expand Up @@ -257,7 +261,15 @@ impl GithubCore {

match resp.status() {
StatusCode::OK => Ok(()),
StatusCode::NOT_FOUND if args.if_match().is_some() => Err(Error::new(
ErrorKind::ConditionNotMatch,
"delete precondition requires a live target",
)),
StatusCode::NOT_FOUND => Ok(()),
StatusCode::CONFLICT if args.if_match().is_some() => Err(Error::new(
ErrorKind::ConditionNotMatch,
"delete sha does not match the current blob",
)),
_ => Err(parse_error(resp)),
}
}
Expand Down Expand Up @@ -412,7 +424,7 @@ mod error {
let (kind, retryable) = match parts.status.as_u16() {
401 | 403 => (ErrorKind::PermissionDenied, false),
404 => (ErrorKind::NotFound, false),
304 | 412 => (ErrorKind::ConditionNotMatch, false),
304 | 409 | 412 => (ErrorKind::ConditionNotMatch, false),
// https://github.com/apache/opendal/issues/4146
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/423
// We should retry it when we get 423 error.
Expand Down Expand Up @@ -450,14 +462,24 @@ mod error {

#[tokio::test]
async fn test_parse_error() {
let err_res = vec![(
r#"{
let err_res = vec![
(
r#"{
"message": "Not Found",
"documentation_url": "https://docs.github.com/rest/repos/contents#get-repository-content"
}"#,
ErrorKind::NotFound,
StatusCode::NOT_FOUND,
)];
ErrorKind::NotFound,
StatusCode::NOT_FOUND,
),
(
r#"{
"message": "is at abc123 but expected def456",
"documentation_url": "https://docs.github.com/rest/repos/contents#delete-a-file"
}"#,
ErrorKind::ConditionNotMatch,
StatusCode::CONFLICT,
),
];

for res in err_res {
let bs = bytes::Bytes::from(res.0);
Expand Down
7 changes: 2 additions & 5 deletions core/services/github/src/deleter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,7 @@ impl GithubDeleter {
}

impl oio::OneShotDelete for GithubDeleter {
async fn delete_once(&self, path: String, _: OpDelete) -> Result<()> {
match self.core.delete(&self.ctx, &path).await {
Ok(_) => Ok(()),
Err(err) => Err(err),
}
async fn delete_once(&self, path: String, args: OpDelete) -> Result<()> {
self.core.delete(&self.ctx, &path, &args).await
}
}
5 changes: 5 additions & 0 deletions core/services/github/src/docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ Depending on its configuration and the backing system, this service can expose:
Inspect the effective capability set with [`opendal_core::Operator::info`] and
[`opendal_core::OperatorInfo::capability`] after building an operator.

Conditional delete with `if_match` sends the blob SHA from
`Metadata::etag` as the Contents API `sha`. A mismatched SHA returns
`ErrorKind::ConditionNotMatch` and leaves the file in place. Unconditional
delete still looks up the current SHA first.

## Configuration

Use [`crate::GithubConfig`] for serializable configuration and this builder's
Expand Down
Loading