-
Notifications
You must be signed in to change notification settings - Fork 2
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
wip: Alternative Storage Target support #190
Conversation
Extracted a base interface |
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.
We miss test-suite/src/test/java/io/micronaut/microstream/docs/PostgresPersistentCacheTest.java and test-suite/src/test/java/io/micronaut/microstream/docs/PostgresCustomerControllerTest.java for S3
...ostream/src/main/java/io/micronaut/microstream/s3/DefaultS3StorageConfigurationProvider.java
Outdated
Show resolved
Hide resolved
...ostream/src/main/java/io/micronaut/microstream/s3/DefaultS3StorageConfigurationProvider.java
Show resolved
Hide resolved
...ostream/src/main/java/io/micronaut/microstream/s3/DefaultS3StorageConfigurationProvider.java
Outdated
Show resolved
Hide resolved
@sdelamo I believe this is ready for review 🤞 |
microstream/src/main/java/io/micronaut/microstream/s3/S3StorageConfigurationProvider.java
Outdated
Show resolved
Hide resolved
@Bean | ||
@Singleton | ||
@Named(OTHER_CLIENT_NAME) | ||
S3Client buildClient() { |
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 is more correct to pass to the factory method the bean dependency instead of to the factory constructor because it clearly defines the bean hierarchy.
S3Client buildClient() { | |
S3Client buildClient(S3Config s3Config) { |
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.
I have changed it here: 350e6d5
…eConfigurationProvider.java
@timyates I have flagged the tests which require docker and extract some test utils |
test-suite-utils/src/main/java/io/microstream/testutils/LocalStackUtils.java
Outdated
Show resolved
Hide resolved
SonarCloud Quality Gate failed. |
No description provided.