Conversation
| throw new IllegalArgumentException("Not supported s3StorageClass: null"); | ||
| } | ||
| try { | ||
| return S3StorageClass.valueOf(s3StorageClass.toUpperCase()); |
There was a problem hiding this comment.
Thanks @devmadhuu.
I wonder would Locale.ROOT be safer here? toUpperCase() uses the JVM's default locale.
For example, under Turkish locale, "standard_ia" becomes "STANDARD_İA", which does not match the enum constant.
The Java documentation recommends Locale.ROOT for locale-independent strings such as protocol keys.
Gargi-jais11
left a comment
There was a problem hiding this comment.
Thanks @devmadhuu for this PR.
| // S3 storage class values passed via the `x-amz-storage-class` request header. | ||
| public static final String S3_STORAGE_CLASS_STANDARD = "STANDARD"; | ||
| public static final String S3_STORAGE_CLASS_STANDARD_IA = "STANDARD_IA"; | ||
| public static final String S3_STORAGE_CLASS_GLACIER = "GLACIER"; | ||
|
|
There was a problem hiding this comment.
I don't know whether u are aware or not but S3StorageType.java in the S3 Gateway has similar enums means: “this S3 storage class string picks how many copies / EC layout the object gets” (STANDARD → 3-way Ratis, STANDARD_IA → EC, etc.). Almost all upload/download code uses that today.
This PR adds S3StorageClass, which means: “this string picks Hot / Warm / Cold (which disk tier).”
Those are two different ideas with similar names. That will confuse people and cause wrong imports later.
We should look into these and check how they both can be used independently.
There was a problem hiding this comment.
AWS's StorageClass https://aws.amazon.com/s3/storage-classes does not explicitly specify the replica types or storage media types for each StorageClass; instead, it defines concepts such as durability, SLA, zones, and latency , etc for different StorageClasses
Ozone’s S3 StorageClasses can have custom definitions; users can specify the storage medium (storage policy) and replica type through S3 StorageClass parameter, which does not conflict with AWS S3’s StorageClasses.
| try { | ||
| return S3StorageClass.valueOf(s3StorageClass.toUpperCase()); | ||
| } catch (IllegalArgumentException e) { | ||
| throw new IllegalArgumentException( |
There was a problem hiding this comment.
IllegalArgumentException is fine for a util with no callers yet but PutObject must map unknown classes to OS3Exception / InvalidStorageClass, not a 500.
What changes were proposed in this pull request?
This PR is to introduce
S3StorageClass— a small enum that maps AWS S3 storage class strings (STANDARD,STANDARD_IA,GLACIER) to Ozone storage policies (HOT,WARM,COLD).Two helper methods on the enum:
fromS3StorageClass(String)— parse the value of an incomingx-amz-storage-classheader.fromStoragePolicy(StoragePolicy)— go the other way, for building responses.This PR is infrastructure only — no caller uses
S3StorageClassyet. The S3 write path (PutObject) and read path (GetObject) will start calling it in follow-up PRs.What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-16039
How was this patch tested?
New unit test
TestS3StorageClasscovers:nulland unknown values.StoragePolicy→ enum for all three, plus rejectsnull.