Conversation
cfc6064 to
88af5ac
Compare
maciejwalkowiak
left a comment
There was a problem hiding this comment.
Thanks guys for a PR! I left few comments after quick look. I'll wait with deeper review till you make it non-draft..? Or is there something specific you would like to get feedback on before?
Hint: run make format before pushing or build with make build to avoid having CrossRegionS3Client modified.
...s-autoconfigure/src/main/java/io/awspring/cloud/autoconfigure/core/AwsAutoConfiguration.java
Outdated
Show resolved
Hide resolved
...src/main/java/io/awspring/cloud/autoconfigure/core/CloudWatchMetricsPublisherProperties.java
Outdated
Show resolved
Hide resolved
...src/main/java/io/awspring/cloud/autoconfigure/core/CloudWatchMetricsPublisherProperties.java
Outdated
Show resolved
Hide resolved
...configure/src/main/java/io/awspring/cloud/autoconfigure/core/AwsClientBuilderConfigurer.java
Outdated
Show resolved
Hide resolved
...-aws-autoconfigure/src/main/java/io/awspring/cloud/autoconfigure/s3/S3AutoConfiguration.java
Outdated
Show resolved
Hide resolved
|
Hey @maciejwalkowiak its just me although it shows 2 people (changed laptops and forgot to change my git user to push a couple commits, will amend that before removing the draft status) Still working on it, tests are failing here because no region provider (worked on my machine because I have a default REGION set locally). Will work on that, fix the things you mentioned and add the parameterstore and secretmanager because they are not following the normal autoconfiguration pattern. The PR is a draft as I'm still workign on it, will notify when ready to review, just wanted to open it to show that it's actively worked on |
82b2ebb to
91a64bd
Compare
|
Thanks @krimsz! Keep it going, great work and ping me in case you have any questions. |
|
Kudos, SonarCloud Quality Gate passed!
|
|
@maciejwalkowiak if you can please take a look at around how I'm creating (probably in a very wrong way) at My main issue with the approach here (besides the ugliness since I don't have access to the full context with full bean creation) is how to test it properly. The way the clients hide the builder makes it impossible to access the metricsPublisher at all |








📢 Type of change
📜 Description
Enable CloudWatch metrics if dependency is in the path
💡 Motivation and Context
Enhancement #345
💚 How did you test it?
In progress...
📝 Checklist
In progress...
🔮 Next steps
Still a draft, wip