add support to set a prefix for the S3 key #118
Conversation
| @@ -32,6 +33,7 @@ public class ExtendedClientConfiguration extends PayloadStorageConfiguration { | |||
| private boolean cleanupS3Payload = true; | |||
| private boolean useLegacyReservedAttributeName = true; | |||
| private boolean ignorePayloadNotFound = false; | |||
| private String s3KeyPrefix; | |||
There was a problem hiding this comment.
Do you think we should consider checking the validness of the the s3KeyPrefix? e.g.
- The key name must be between 1 and 1024 bytes in length.
- some special characters may not be allowed or may have restrictions. For example, AWS S3 recommends avoiding characters like '/', '', and '@' because they can have special meanings in certain contexts.
- While you can use periods (".") in key names, you should avoid having a key name that starts with a period. Some tools and applications may not handle such key names well.
- AWS S3 has some reserved words that you should avoid using as key names, such as '.' and '..'.
There was a problem hiding this comment.
I think we can add these validations. However, the / character is the only one I think we should allow because the AWS console will show the objects like a SO directory, which makes it easier to see messages of a specific queue or use case.
There was a problem hiding this comment.
Yes, i think allowing / should be fine. Originally the s3 recommended to not use it is due to performance drawback.
There was a problem hiding this comment.
Do you think for this feature make sense allow only characters that matches with the regex [a-zA-Z0-9-_\/\.]? According to the S3 doc there many characters that we should avoid or requires special handling, and I think it should not be responsability of the amazon-sqs-java-extended-client lib handle this.
There was a problem hiding this comment.
Yes, taking every single corner case into consideration jumps out of the scope of this PR. I like your suggestion. One minor comment to that regex pattern is that
the hyphen is treated literally, which might be confusing for others. Consider escaping it or placing at the start or end of the string.
[a-zA-Z0-9\-\_\/\.]
*Issue #7 *
Description of changes:
s3KeyPrefix) for the ExtendedClientConfiguration class that will be used to compose the s3 key.queue-name/, the S3 key will be something likequeue-name/676468c5-671f-4471-b9f6-fc5b0a5ab7de.payloadStore.storeOriginalPayload(messageContentStr, s3Key)will be used only if the s3KeyPrefix is not blank.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.