Consider hiding pubSubReactiveScheduler in Pub/Sub autoconfig
elefeint opened this issue · 9 comments
The scheduler is a cached scheduler, which implements a Supplier
. Since Spring Cloud Stream autodiscovers functional beans, it always finds pubSubReactiveScheduler
, meaning that any SCS app that uses the Cloud Pub/Sub binder will not be able to take advantage of autodiscovery and will have to specify spring.cloud.function.definition
property to disambiguate.
@artembilan What would you recommend we do here?
I don't think this issue is still valid. As long as your @Bean
definition explicit type is not a Supplier
, Consumer
or Function
, this bean is not going to be discovered by Spring Cloud Function logic.
I'm not sure why you bring this issue again since it has been fixed in Spring Cloud Function some time ago: spring-cloud/spring-cloud-function#409
This bean is secretly a Supplier
. It no longer breaks the build after the fix, but it still exists in the context, so applications that actually want this SCS behavior can't get the default "single functional bean exists in context, therefore just use that" behavior.
Sorry, @elefeint , you last comment is not clear to me.
We don't need to fix anything in this project since that was fixed in the Spring Cloud Function. Please, let's stop talk about Spring Cloud Stream: functions is not its responsibility.
Even if your bean essentially implement Supplier
somehow, it is not present in the function catalog, therefore any function magic is not applier to it.
One more time:
- bean is a function catalog candidate only when its definition is explicitly assignable from
Supplier
,Consumer
orFunction
.
It doesn't look likeScheduler
interface has one of those in its declaration. - Only functions from the catalog are treated as Spring Cloud Functions with an appropriate adaptation to Lambda, REST, Spring Cloud Stream etc.
- So, even you you call
BeanFactory.getBeansOfType(Supplier.class)
, it doesn't mean that you are going to get the same set as you would callFunctionCatalog.getNames(Supplier.class)
But that's IMHO, you can do with this issue whatever you think is sufficient.
Thanks
That's fair; both the spring.cloud.function.definition
property and the autodiscovery logic are within Spring Cloud Function's purview. I keep mentioning Spring Cloud Stream because the current usecase (illustrated with a new sample app in #1990) happens to be with any Spring Cloud Stream app that uses our Pub/Sub binder.
For 1: pubSubReactiveScheduler
bean is instantiated as a Schedulers.elastic()
, which returns a CachedScheduler
. And CachedScheduler
implements Supplier, giving us our problem!
For 2, in Spring Cloud Functions, the scheduler supplier is identified as a valid Supplier
. From here, there are two scenarios:
Scenario A: there are no functional beans in the user application itself (this is not an application that meant to use function autodiscovery at all). This is the scenario that used to cause us build failures, and that's what got fixed by the additional parameter validation. SCF will output a message "Discovered functional instance of bean 'pubSubReactiveScheduler' as a default function, however its function argument types can not be determined. Discarding.", so there is no issue.
Scenario B: there is a functional bean in the user app that's supposed to be used (the Spring Cloud Stream functional sample app is a good example).
In this case, SCF will print this message and not populate the function registry at all (which is not what was intended in the user app): "Found more then one function beans in BeanFactory: [logUserMessage, pubSubReactiveScheduler]. If you did not intend to use functions, ignore this message."
So, scenario A (total breakage when another functional bean exists) was fixed, but scenario B (inconvenience of not having autodiscovered functions) is still present.
Does your explanation mean that the fix for spring-cloud/spring-cloud-function#409 is not full or there is something missed?
Are you sure that you use the latest Spring Cloud Function in your sample?
/CC @olegz
The fix was good for addressing the severe issue of the scheduler bean being misidentified as a valid SCF target.
It could perhaps be enhanced by removing such beans from even becoming candidates for autodiscovery. For example, this validation block of code could become a filter to be applied while aggregating functions, before the checks for the number of autodiscovered functions is done.
Sounds like plan to go!
I really don't see reason to pursue some workaround here, when we definitely could fix upstream.
Let's raise an issue in Spring Cloud Function and see what @olegz thinks about that!
Will do. I'll commit the sample first, so we have something to talk about.
My original plan was to ask for a new @DisableFunctionalAutodiscovery
annotation in Spring Cloud Function, but perhaps that is not actually necessary if only the true functional beans are discovered.