Could anyone trigger the build again please?
It seems failing test is not related to my changes.

Codecov Report

Merging #3663 into master will decrease coverage by 0.07%.
The diff coverage is 87.45%.

@@            Coverage Diff             @@
##           master    #3663      +/-   ##
==========================================
- Coverage   74.49%   74.41%   -0.08%     
==========================================
  Files         126      126              
  Lines        5990     5996       +6     
  Branches      390      393       +3     
==========================================
  Hits         4462     4462              
- Misses       1528     1534       +6

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 22c0807...a3c21b2. Read the comment docs.

@style95
Closing the PR and then re-opening the PR will trigger the build again in Travis

@csantanapr thank you for the good tip.

@tysonnorris suggested to use a single SPI implementation for ContainerProxy and ContainerPool.

At first time, I thought of this kind of approach.

trait ContainerPoolProvider extends Spi {
  def getContainerPoolProps(childFactory: ActorRefFactory => ActorRef,
            poolConfig: ContainerPoolConfig,
            activationFeed: ActorRef,
            someConfig: Option[PrewarmingConfig] = None): Props

  def getContainerProxyProps(factory: (TransactionId, String, ImageName, Boolean, ByteSize, Int) => Future[Container],
            ack: (TransactionId, WhiskActivation, Boolean, InstanceId, UUID) => Future[Any],
            store: (TransactionId, WhiskActivation) => Future[Any],
            collectLogs: (TransactionId, Identity, WhiskActivation, Container, ExecutableWhiskAction) => Future[ActivationLogs],
            instance: InstanceId,
            poolConfig: ContainerPoolConfig,
            unusedTimeout: FiniteDuration = timeouts.idleContainer,
            pauseGrace: FiniteDuration = timeouts.pauseGrace): Props

  def requiredProperties: Map[String, String]
}

So we can create both ContainerProxy and ContainerPool implementation with one trait.
But with this approach, companion object of them should mix in the same trait, ContainerPoolProvider.
Since that trait has two abstract methods, it is required for all objects which mix in this trait to override both methods.
Accordingly ContainerProxy and ContainerPool should override unnecessary methods respectively.

One option is to make ContainerProxy as a inner class of ContainerPool and only expose ContainerPool via SPI.
But it might introduce huge changes.

As of now I am not quite sure which one is right direction.
I am holding to have multiple SPI for them in one hand, and to make ContainerProxy as an inner class of ContainerPool in the other hand.

WDYT?

Discalimer: i need to catch up on the tech exchange and various other write ups. But at first glance two adapters avoids conflating separate concerns. I do not see why they should be mixed or nested.

The basic idea behind above approach is, ContainerProxy and ContainerPool have strong correlation to each other.
I am not sure we can independently use ContainerProxy or ContainerPool.
Logics in ContainerProxy do affect the scheduling and pool managing logics in ContainerPool.
So I thought if we make ContainerProxy as an inner class of ContainerPool then we can have better cohesion.

@style95 this has rotted quite a bit and the dev-list discussions seem to move in another direction. I'll close this for now due to inactivity. Please feel free to reopen it if a proposal going this way is discussed and agreed upon on the dev-list.

Thanks a lot for your work so far, very appreciated! 🎉

@markusthoemmes got it.
Thanks.