fix: ensure versionPromise always completes #1155
Conversation
| import org.scalatest.BeforeAndAfterEach | ||
|
|
||
| import scala.concurrent.duration._ | ||
|
|
There was a problem hiding this comment.
code here moved to KubernetesApiSpec because it's only testing KubernetesApi.
AppVersionRevisionSpec is not testing the failure case inside AppVersionRevision.start
| s"Be sure to provide the pod name with `$configPath.pod-name` " + | ||
| "or by setting ENV variable `KUBERNETES_POD_NAME`." | ||
| log.error(msg) | ||
| versionPromise.failure(new IllegalStateException(msg)) |
There was a problem hiding this comment.
If podname is not set, there is nothing else we can do, but if we try to later use getRevision we get a Future that never completes.
There was a problem hiding this comment.
Hmm, maybe we should not return an IllegalStateException, because it's not illegal. It's just missing info causing a degraded state.
| class AppVersionRevisionSpec | ||
| extends TestKit( | ||
| ActorSystem( | ||
| "AppVersionRevisionSpec", | ||
| AppVersionRevisionSpec.config | ||
| KubernetesApiSpec.config |
There was a problem hiding this comment.
yep, that's wrong. Copy-n-pasta wrong.
There was a problem hiding this comment.
Fixed.
It turns out that tests were passing even without using the right config. Which makes sense because podname defaults to empty in the reference.conf.
Anyway, I keep the new config just to make it very explicit, but did some clean-up as well.
octonato
force-pushed
the
octonato/complete-revision-promise-on-failure
branch
from
d02414b
to
04681f8
Compare
Noticed today with @franciscolopezsancho, that the versionPromise may never complete forcing users to do some weird future gymnastics to work it around.