tustvold commented on code in PR #4834:
URL: https://github.com/apache/arrow-rs/pull/4834#discussion_r1329851219
##########
object_store/src/client/header.rs:
##########
@@ -19,11 +19,33 @@
use crate::path::Path;
use crate::ObjectMeta;
-use chrono::{DateTime, Utc};
+use chrono::{DateTime, TimeZone, Utc};
use hyper::header::{CONTENT_LENGTH, ETAG, LAST_MODIFIED};
use hyper::HeaderMap;
use snafu::{OptionExt, ResultExt, Snafu};
+#[derive(Debug)]
+/// Configuration for header extraction
+pub struct HeaderConfig {
+ /// Whether to require an ETag header when extracting [`ObjectMeta`] from
headers.
+ ///
+ /// Defaults to `true`
+ pub etag_required: bool,
+ /// Whether to require a Last-Modified header when extracting
[`ObjectMeta`] from headers.
+ ///
+ /// Defaults to `true`
+ pub last_modified_required: bool,
+}
+
+impl Default for HeaderConfig {
Review Comment:
FWIW you could avoid this and just `derive(Default)` if you the members were
`last_modified_optional` instead of `required`
##########
object_store/src/client/header.rs:
##########
@@ -52,32 +74,55 @@ pub enum Error {
}
/// Extracts [`ObjectMeta`] from the provided [`HeaderMap`]
-pub fn header_meta(location: &Path, headers: &HeaderMap) -> Result<ObjectMeta,
Error> {
- let last_modified = headers
- .get(LAST_MODIFIED)
- .context(MissingLastModifiedSnafu)?;
+pub fn header_meta(
+ location: &Path,
+ headers: &HeaderMap,
+ cfg: HeaderConfig,
+) -> Result<ObjectMeta, Error> {
+ let last_modified = match headers.get(LAST_MODIFIED) {
+ Some(last_modified) => {
+ let last_modified =
last_modified.to_str().context(BadHeaderSnafu)?;
+ let last_modified = DateTime::parse_from_rfc2822(last_modified)
+ .context(InvalidLastModifiedSnafu { last_modified })?
+ .with_timezone(&Utc);
+ last_modified
+ }
+ None => {
+ if cfg.last_modified_required {
+ return Err(Error::MissingLastModified);
+ } else {
+ chrono::Utc.timestamp_nanos(0)
+ }
+ }
+ };
+
+ let e_tag = match headers.get(ETAG) {
+ Some(e_tag) => {
+ let e_tag = e_tag.to_str().context(BadHeaderSnafu)?;
+ Some(e_tag.to_string())
+ }
+ None => {
+ if cfg.etag_required {
+ return Err(Error::MissingEtag);
+ } else {
+ None
+ }
+ }
Review Comment:
```suggestion
None if cfg.etag_required => return Err(Error::MissingEtag),
None => None,
```
##########
object_store/src/client/header.rs:
##########
@@ -52,32 +74,55 @@ pub enum Error {
}
/// Extracts [`ObjectMeta`] from the provided [`HeaderMap`]
-pub fn header_meta(location: &Path, headers: &HeaderMap) -> Result<ObjectMeta,
Error> {
- let last_modified = headers
- .get(LAST_MODIFIED)
- .context(MissingLastModifiedSnafu)?;
+pub fn header_meta(
+ location: &Path,
+ headers: &HeaderMap,
+ cfg: HeaderConfig,
+) -> Result<ObjectMeta, Error> {
+ let last_modified = match headers.get(LAST_MODIFIED) {
+ Some(last_modified) => {
+ let last_modified =
last_modified.to_str().context(BadHeaderSnafu)?;
+ let last_modified = DateTime::parse_from_rfc2822(last_modified)
+ .context(InvalidLastModifiedSnafu { last_modified })?
+ .with_timezone(&Utc);
+ last_modified
+ }
+ None => {
+ if cfg.last_modified_required {
+ return Err(Error::MissingLastModified);
+ } else {
+ chrono::Utc.timestamp_nanos(0)
+ }
+ }
Review Comment:
```suggestion
None if cfg.last_modified_required => return
Err(Error::MissingLastModified),
None => Utc.timestamp_nanos(0),
```
##########
object_store/src/client/header.rs:
##########
@@ -52,32 +74,55 @@ pub enum Error {
}
/// Extracts [`ObjectMeta`] from the provided [`HeaderMap`]
-pub fn header_meta(location: &Path, headers: &HeaderMap) -> Result<ObjectMeta,
Error> {
- let last_modified = headers
- .get(LAST_MODIFIED)
- .context(MissingLastModifiedSnafu)?;
+pub fn header_meta(
+ location: &Path,
+ headers: &HeaderMap,
+ cfg: HeaderConfig,
+) -> Result<ObjectMeta, Error> {
+ let last_modified = match headers.get(LAST_MODIFIED) {
+ Some(last_modified) => {
+ let last_modified =
last_modified.to_str().context(BadHeaderSnafu)?;
+ let last_modified = DateTime::parse_from_rfc2822(last_modified)
+ .context(InvalidLastModifiedSnafu { last_modified })?
+ .with_timezone(&Utc);
+ last_modified
Review Comment:
```suggestion
DateTime::parse_from_rfc2822(last_modified)
.context(InvalidLastModifiedSnafu { last_modified })?
.with_timezone(&Utc)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
