|
6 | 6 |
|
7 | 7 | ### Closed Decisions
|
8 | 8 |
|
9 |
| -1. **Response Type**: Use existing `GetObjectResponse` (HTTP response identical to standard S3 GetObject) |
10 |
| -2. **Exception Handling**: Utilize existing SDK exceptions rather than specialized ones |
11 |
| -3. **Validation Strategy**: No additional client-side validation for pre-signed URLs |
| 9 | +1. Create a new PresignedUrlGetObjectResponse specifically for pre-signed URLs, or use the existing GetObjectResponse? Decided to use the existing GetObjectResponse for pre-signed URL operations as the HTTP response from a pre-signed URL GET is same as a standard S3 GetObject response. |
| 10 | + |
| 11 | +2. Use the existing SDK and S3 exceptions or implement specialized exceptions for validation errors like expired URLs? Decided to utilize existing SDK exceptions rather than creating specialized ones for pre-signed URL operations. |
| 12 | + |
| 13 | +3. Provide additional client-side validation with server-side validation as fallback or just rely entirely on server-side validation from S3? No additional client-side validation will be implemented for pre-signed URLs. |
12 | 14 |
|
13 | 15 | ### Discussions Addressed
|
14 | 16 |
|
15 |
| -1. **Credential Signing Bypass**: Added `NoOpSigner()` usage in design |
16 |
| -2. **Checksum Support**: S3 response doesn't include checksum for presigned URLs |
17 |
| -3. **Helper API Naming**: Options include `PresignedURLManager` vs `PresignedUrlExtension` (TBD in Surface API Review) |
| 17 | +1. Are there alternative methods to skip signing, such as using NoOpSigner(), instead of setting additional Execution attributes? Added the use of NoOpSigner() in the design doc. |
| 18 | + |
| 19 | +2. Does the S3 response include a checksum? If so, should checksum-related support be implemented in this project, or deferred until after Transfer Manager support is added? S3 Response doesn't include checksum. |
| 20 | + |
| 21 | +3. What should we name the Helper API? Options include PresignedURLManager or PresignedUrlExtension. Will be addressed in the Surface API Review. |
18 | 22 |
|
19 | 23 | ## Review Meeting: 06/23/2024
|
20 | 24 | **Attendees**: John, Zoe, Dongie, Bole, Ran, Saranya, Alex, David
|
21 | 25 |
|
22 | 26 | ### Decisions Addressed
|
23 | 27 |
|
24 |
| -1. **Request Design Pattern**: Use standalone request class with minimal parameters (presignedUrl, rangeStart, rangeEnd) to avoid exposing incompatible configurations. Convert internally to S3Request for ClientHandler compatibility. |
| 28 | +1. Should PresignedUrlGetObjectRequest extend S3Request/SdkRequest? Decided to use a standalone request class with minimal parameters (presignedUrl, rangeStart, rangeEnd) to avoid exposing incompatible configurations like credentials and signers. Internally convert to S3Request for ClientHandler compatibility. |
25 | 29 |
|
26 |
| -2. **Endpoint Resolution Bypass**: Introduce new `SKIP_ENDPOINT_RESOLUTION` execution attribute specifically for presigned URL scenarios, replacing `IS_DISCOVERED_ENDPOINT` which is tied to deprecated endpoint discovery feature. |
| 30 | +2. Replace IS_DISCOVERED_ENDPOINT execution attribute with a more semantically appropriate solution. Decided to introduce new SKIP_ENDPOINT_RESOLUTION execution attribute specifically for presigned URL scenarios where endpoint resolution should be bypassed, as IS_DISCOVERED_ENDPOINT is tied to deprecated endpoint discovery feature. |
27 | 31 |
|
28 |
| -3. **Range Parameter Design**: Use separate `rangeStart` and `rangeEnd` Long fields for better user experience over string parsing. |
| 32 | +3. Use separate rangeStart/rangeEnd fields vs single range string parameter. Decided to use separate rangeStart and rangeEnd Long fields for better user experience, as start/end is more intuitive than string parsing. |
0 commit comments