-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
add static factory for custom S3 endpoint in S3ObjectMonitor #171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
|
@alexsunday Apologies for the delay here; let's reopen and review this. |
|
Perhaps the original intention of Signal's open-source release was to enable code review for verifying its security, but in reality, very few people independently follow up and deploy the latest version of the server software? These changes are critical for self-deployment, while maintaining excessive patches and branches independently becomes overly complex. Therefore, I hope they can be merged. Thank you. |
...main/java/org/whispersystems/textsecuregcm/configuration/MonitoredS3ObjectConfiguration.java
Show resolved
Hide resolved
| // allows specifying a custom S3 endpoint | ||
| static public S3ObjectMonitor createCustomS3(final AwsCredentialsProvider awsCredentialsProvider, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like we're mixing metaphors here; this would mean we have a mix of constructors and static create methods. I think we should stick to one or the other.
Although reasonable minds might disagree, I'd suggest sticking with overloaded constructors since that's what's already happening here (even if it is only for a visible-for-testing constructor). Specifically, I'd introduce a second public constructor that accepts an s3Endpoint argument, then have the last (package-private) constructor in the chain conditionally modify the S3ClientBuilder depending on the presence of the s3Endpoint argument.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It has been changed to use the overloaded constructor style. Please review it again.
|
How do I submit code to your repository to add support for supergroups and superchannels? |
|
Thank you for the contribution! |
This change add a static method createCustomS3 to S3ObjectMonitor, allowing the creation of S3 clients with custom endpoints. This is useful for development and testing environments, enabling Signal to interact with local or non-standard S3-compatible storage. The method supports endpoint override and path-style access, making it easier to run integration tests without relying on AWS infrastructure.