[GitHub] [incubator-openwhisk] style95 commented on a change in pull request #4453: Add SPI for invoker

[GitBox <gi...@apache.org>]

style95 commented on a change in pull request #4453: Add SPI for invoker
URL: https://github.com/apache/incubator-openwhisk/pull/4453#discussion_r277263031
 
 

 ##########
 File path: core/invoker/src/main/scala/org/apache/openwhisk/core/invoker/Invoker.scala
 ##########
 @@ -156,25 +156,50 @@ object Invoker {
           .isFailure) {
       abort(s"failure during msgProvider.ensureTopic for topic $topicName")
     }
+
     val producer = msgProvider.getProducer(config, Some(ActivationEntityLimit.MAX_ACTIVATION_LIMIT))
     val invoker = try {
-      new InvokerReactive(config, invokerInstance, producer, poolConfig)
+      SpiLoader.get[InvokerProvider].instance(config, invokerInstance, producer, poolConfig, limitConfig)
 
 Review comment:
   I considered many options to apply SPI in invoker.
   For example, it would be possible to keep `InvokerReactive` and apply SPI to `ContainerPool` and `ContainerProxy` level.
   But I noticed that they are also highly dependent on `InvokerReactive`.
   So it would be more natural to apply SPI at the highest level of implementation.
   It will abstract and hide the fundamental implementation such as `ContainerPool` and `ContainerProxy` from the top level Invoker implementation.
   
   Also, there could be some dependent RestAPI implementations corresponding to the Invoker implementation.
   So I added SPI provider for `InvokerServer` which provides REST API as well.
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services