aws-sdk-rust icon indicating copy to clipboard operation
aws-sdk-rust copied to clipboard

S3 conditional GETs should return an easier to manage error

Open dialtone opened this issue 3 years ago • 1 comments
trafficstars

Describe the feature

If one wants to perform a conditional GET from S3, it seems the current pattern is:

    let client = Client::new(&shared_config);
    let resp1 = client
        .get_object()
        .bucket(bucket.to_string())
        .key(from.to_string())
        .set_if_modified_since(last_modified)
        .send()
        .await;

    if let Err(e) = resp1 {
        match e {
            SdkError::ServiceError {
                err: _,
                raw: ref ra,
            } => {
                if ra.http().status() == 304 {
                    tracing::info!("not modified");
                    return Ok(last_modified);
                }
                return Err(e.into());
            }
            _ => return Err(e.into()),
        }
    } else {
        let mut resp = resp1?;
        let tmp = to.as_ref().to_path_buf().with_extension("tmp");
        if let Some(parent) = to.as_ref().parent() {
            tokio::fs::create_dir_all(parent).await?;
        }
        {
            let mut file = File::create(tmp.as_path()).await?;
            while let Some(bytes) = resp.body.next().await {
                let bytes = bytes?;
                file.write_all(&bytes).await?;
            }
            file.flush().await?;
        }
        tokio::fs::rename(tmp, to).await?;
        info!("Downloaded");
        Ok(resp.last_modified().cloned())
    }

Use Case

More intuitively handle the conditional S3 GET. Maybe there's already some way to handle this that I missed.

Proposed Solution

It seems to me that perhaps ServiceError should provide an API to quickly determine if it's a NotModified error, also perhaps adding NotModified variant to GetObjectErrorKind since right now this falls under Unhandled which requires to go deeper into the raw Response.

Other Information

No response

Acknowledgements

  • [ ] I may be able to implement this feature request
  • [ ] This feature might incur a breaking change

A note for the community

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue, please leave a comment

dialtone avatar May 14 '22 21:05 dialtone

Thanks for filing this! I'll escalate this to our internal team that is responsible for updating the S3 model.

@jmklix - P62314171

jdisanti avatar May 16 '22 21:05 jdisanti

Opened an issue in aws/aws-sdk which is used to track issues across multiple sdks. Closing this issue

jmklix avatar Oct 03 '22 16:10 jmklix

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.

github-actions[bot] avatar Oct 03 '22 16:10 github-actions[bot]