stop ClusterClient ResponseTunnel after first reply when ask is used, #22093 #22094

Merged
merged 1 commit into from Jan 10, 2017

Projects

None yet

3 participants

@patriknw
Member
patriknw commented Jan 3, 2017

Refs #22093

@akka-ci
Member
akka-ci commented Jan 3, 2017

Test FAILed.

@akka-ci akka-ci removed the validating label Jan 3, 2017
@patriknw
Member
patriknw commented Jan 3, 2017

created issue for unrelated failure: #22097

@patriknw
Member
patriknw commented Jan 3, 2017

PLS BUILD

@akka-ci
Member
akka-ci commented Jan 3, 2017

Test PASSed.

@@ -812,12 +812,18 @@ object ClusterReceptionist {
*/
class ClientResponseTunnel(client: ActorRef, timeout: FiniteDuration) extends Actor with ActorLogging {
context.setReceiveTimeout(timeout)
+
+ private val isAsk = client.path.elements.headOption.contains("temp")
@ktoso
ktoso Jan 5, 2017 Member

maybe client.path.toString startsWith "/temp/$ would be a bit future-safer?
I was checking if we don't register FunctionRefs here but seems we don't... Do you think this is something to worry about or not really? Not sure if Typed needs to register temp actors hm

@patriknw
patriknw Jan 5, 2017 Member

ok, we can add the additional check that the second element starts with "$"
(I prefer to not do full toString)

@ktoso
ktoso approved these changes Jan 5, 2017 View changes

LGTM, somewhat iffy about the temp checking though I see that it currently works.

@patriknw patriknw stop ClusterClient ResponseTunnel after first reply when ask is used, #…
b213a7a
@ktoso
ktoso approved these changes Jan 5, 2017 View changes

LGTM

+ private val isAsk = {
+ val pathElements = client.path.elements
+ pathElements.size == 2 && pathElements.head == "temp" && pathElements.tail.head.startsWith("$")
+ }
@ktoso
ktoso Jan 5, 2017 Member

Cool, this seems more future ready 👍

@ktoso ktoso added the reviewed label Jan 6, 2017
@patriknw
Member
patriknw commented Jan 9, 2017

PLS BUILD

@akka-ci
Member
akka-ci commented Jan 9, 2017

Test PASSed.

@patriknw patriknw added this to the 2.5.0 milestone Jan 10, 2017
@patriknw patriknw merged commit 1eba865 into master Jan 10, 2017

2 checks passed

default Test PASSed.
Details
typesafe-cla-validator All users have signed the CLA
Details
@patriknw patriknw deleted the wip-22093-ClusterClient-ask-patriknw branch Jan 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment