#1457-Introduce-kinesis-autoconfiguration#1483
#1457-Introduce-kinesis-autoconfiguration#1483MatejNedic wants to merge 16 commits intoawspring:mainfrom
Conversation
MatejNedic
requested review from
maciejwalkowiak and
tomazfernandes
as code owners
maciejwalkowiak
left a comment
maciejwalkowiak
left a comment
There was a problem hiding this comment.
Assuming it is compatible with #1479 we can merge!
But if we are adding Kinesis integration we can also think about:
artembilan
left a comment
artembilan
left a comment
There was a problem hiding this comment.
Just couple hygiene nit-picks.
This affects my PR for spring-cloud-aws-kinesis module only at the dependency .
The KPL/KCL could be auto-configured in other PRs.
Thanks
...gure/src/main/java/io/awspring/cloud/autoconfigure/kinesis/KinesisAsyncClientCustomizer.java
Outdated
Show resolved
Hide resolved
|
Hey @artembilan I will add KPL/KCL auto configuration after I merge yours PR. Will then align dependencies and other stuff. |
...onfigure/src/main/java/io/awspring/cloud/autoconfigure/kinesis/KinesisAutoConfiguration.java
Show resolved
Hide resolved
spring-cloud-aws-starters/spring-cloud-aws-starter-kinesis/pom.xml
Outdated
Show resolved
Hide resolved
...main/java/io/awspring/cloud/autoconfigure/kinesis/KinesisClientLibraryAutoConfiguration.java
Outdated
Show resolved
Hide resolved
|
|
||
| @ConditionalOnMissingBean | ||
| @Bean | ||
| public Scheduler scheduler(ObjectProvider<DynamoDbAsyncClient> dynamoDbClient, |
There was a problem hiding this comment.
This is an active components which requires some lifecycle control.
Plus you cannot guess if end-user is interested only in one of this.
We may go something like SchedulerFactory to hide some common properties, but the rest should be left up to end-user configuration.
This about this like the listener container behind @SqsListener.
And I believe eventually we can come up with something like @KclListener
There was a problem hiding this comment.
I had a same idea for 4.1 to introduce KclListener and work on it! :)
Edit:
Actually thinking about it I will take little bit longer do dig into SqsContainer and KafkaContainer and will do it as part od 4.0.0
There was a problem hiding this comment.
I will take some time reading through Spring Messaging, Spring Kafka and SQS integration. I need more reading on this, but i understand the point behind SchedularFactory and containers. Tnx on constructive feedback Artem, I appreciate it much.
| @ConditionalOnClass({ KinesisAsyncClient.class, Scheduler.class }) | ||
| @EnableConfigurationProperties({ KinesisClientLibraryProperties.class }) | ||
| @AutoConfigureAfter({ CredentialsProviderAutoConfiguration.class, RegionProviderAutoConfiguration.class, | ||
| KinesisAutoConfiguration.class }) |
There was a problem hiding this comment.
Cannot we use CloudWatchExportAutoConfiguration instead of ObjectProvider?
I'm not sure if DynamoDbAsyncClient is the goal in this project yet, though...
There was a problem hiding this comment.
Will move away from ObjectProvider since both DynamoDbAsyncClient and CloudWatchAsync client are not nullabe for Scheduler.
Meaning will have to play little bit with async autoconfiguration in general since most of our clients are sync
# Conflicts: # pom.xml # spring-cloud-aws-kinesis/src/test/java/io/awspring/cloud/kinesis/integration/KinesisIntegrationTests.java
3 participants
📢 Type of change
📜 Description
💡 Motivation and Context
💚 How did you test it?
📝 Checklist
🔮 Next steps