-
Notifications
You must be signed in to change notification settings - Fork 146
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
ScheduleClient for SpringBoot #1816
Conversation
Signed-off-by: Tihomir Surdilovic <tihomir@temporal.io>
...lpha/src/main/java/io/temporal/spring/boot/autoconfigure/RootNamespaceAutoConfiguration.java
Show resolved
Hide resolved
...lpha/src/main/java/io/temporal/spring/boot/autoconfigure/RootNamespaceAutoConfiguration.java
Outdated
Show resolved
Hide resolved
…oral/spring/boot/autoconfigure/RootNamespaceAutoConfiguration.java Co-authored-by: Chad Retz <chad.retz@gmail.com>
Signed-off-by: Tihomir Surdilovic <tihomir@temporal.io>
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.
LGTM, but you may want @Quinn-With-Two-Ns or @mjameswh's review before merging.
TemporalOptionsCustomizer<WorkflowClientOptions.Builder> clientCustomizer) { | ||
TemporalOptionsCustomizer<WorkflowClientOptions.Builder> clientCustomizer, | ||
@Autowired(required = false) @Nullable | ||
TemporalOptionsCustomizer<ScheduleClientOptions.Builder> scheduleCustomize) { |
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.
Question, can you or your tests confirm that, due to type erasure, these last two parameters are properly treated as separate things by Spring?
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.
agree we need customization tests for this. will try to add
LGTM thanks @tsurdilo ! |
Adds support to autowire ScheduleClient.
Schedule apis are not yet implemented for test env so if you have idea what other test can be added please let me know.
Closes #1812