[GitHub] rabbah commented on a change in pull request #3707: enable concurrent runs on ActionContainer test utility

[GitBox]

rabbah commented on a change in pull request #3707: enable concurrent runs on 
ActionContainer test utility 
URL: 
https://github.com/apache/incubator-openwhisk/pull/3707#discussion_r194436470
 
 

 ##
 File path: tests/src/test/scala/actionContainers/ActionContainer.scala
 ##
 @@ -65,13 +67,26 @@ trait ActionProxyContainerTestUtils extends FlatSpec with 
Matchers with StreamLo
   def checkStreams(out: String,
err: String,
additionalCheck: (String, String) => Unit,
-   sentinelCount: Int = 1): Unit = {
+   sentinelCount: Int = 1,
+   concurrent: Boolean = false): Unit = {
 withClue("expected number of stdout sentinels") {
   sentinelCount shouldBe StringUtils.countMatches(out, sentinel)
 }
+//sentinels should be all together
+if (concurrent) {
 
 Review comment:
   Phew. Thanks. 


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


With regards,
Apache Git Services

[GitHub] rabbah commented on a change in pull request #3707: enable concurrent runs on ActionContainer test utility

[GitBox]

rabbah commented on a change in pull request #3707: enable concurrent runs on 
ActionContainer test utility 
URL: 
https://github.com/apache/incubator-openwhisk/pull/3707#discussion_r194295586
 
 

 ##
 File path: tests/src/test/scala/actionContainers/ActionContainer.scala
 ##
 @@ -65,13 +67,26 @@ trait ActionProxyContainerTestUtils extends FlatSpec with 
Matchers with StreamLo
   def checkStreams(out: String,
err: String,
additionalCheck: (String, String) => Unit,
-   sentinelCount: Int = 1): Unit = {
+   sentinelCount: Int = 1,
+   concurrent: Boolean = false): Unit = {
 withClue("expected number of stdout sentinels") {
   sentinelCount shouldBe StringUtils.countMatches(out, sentinel)
 }
+//sentinels should be all together
+if (concurrent) {
 
 Review comment:
   did you remove this change or did I botch the commits?


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


With regards,
Apache Git Services

[GitHub] rabbah commented on a change in pull request #3707: enable concurrent runs on ActionContainer test utility

[GitBox]

rabbah commented on a change in pull request #3707: enable concurrent runs on 
ActionContainer test utility 
URL: 
https://github.com/apache/incubator-openwhisk/pull/3707#discussion_r194294730
 
 

 ##
 File path: common/scala/src/main/scala/whisk/core/containerpool/HttpUtils.scala
 ##
 @@ -53,10 +58,12 @@ import whisk.core.entity.size.SizeLong
  * @param hostname the host name
  * @param timeout the timeout in msecs to wait for a response
  * @param maxResponse the maximum size in bytes the connection will accept
+ * @param maxConcurrent the maximum number of concurrent requests allowed
  */
-protected[core] class HttpUtils(hostname: String, timeout: FiniteDuration, 
maxResponse: ByteSize)(
+protected[core] class HttpUtils(hostname: String, timeout: FiniteDuration, 
maxResponse: ByteSize, maxConcurrent: Int)(
   implicit logging: Logging) {
-
+  def this(hostname: String, timeout: FiniteDuration, maxResponse: 
ByteSize)(implicit logging: Logging) =
 
 Review comment:
   since this makes the constructor public then should the class be open?


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


With regards,
Apache Git Services

[GitHub] rabbah commented on a change in pull request #3707: enable concurrent runs on ActionContainer test utility

[GitBox]

rabbah commented on a change in pull request #3707: enable concurrent runs on 
ActionContainer test utility 
URL: 
https://github.com/apache/incubator-openwhisk/pull/3707#discussion_r194293197
 
 

 ##
 File path: common/scala/src/main/scala/whisk/core/containerpool/HttpUtils.scala
 ##
 @@ -51,9 +56,11 @@ import whisk.core.entity.size.SizeLong
  * @param hostname the host name
  * @param timeout the timeout in msecs to wait for a response
  * @param maxResponse the maximum size in bytes the connection will accept
+ * @param maxConcurrent the maximum number of concurrent requests allowed
  */
-protected[core] class HttpUtils(hostname: String, timeout: FiniteDuration, 
maxResponse: ByteSize) {
-
+protected[core] class HttpUtils(hostname: String, timeout: FiniteDuration, 
maxResponse: ByteSize, maxConcurrent: Int) {
+  def this(hostname: String, timeout: FiniteDuration, maxResponse: ByteSize) =
+this(hostname: String, timeout: FiniteDuration, maxResponse: ByteSize, 1)
 
 Review comment:
   lol @markusthoemmes and i made the same comment i didn't see it sorry.


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


With regards,
Apache Git Services

[GitHub] rabbah commented on a change in pull request #3707: enable concurrent runs on ActionContainer test utility

[GitBox]

rabbah commented on a change in pull request #3707: enable concurrent runs on 
ActionContainer test utility 
URL: 
https://github.com/apache/incubator-openwhisk/pull/3707#discussion_r193919343
 
 

 ##
 File path: tests/src/test/scala/actionContainers/ActionContainer.scala
 ##
 @@ -65,13 +67,26 @@ trait ActionProxyContainerTestUtils extends FlatSpec with 
Matchers with StreamLo
   def checkStreams(out: String,
err: String,
additionalCheck: (String, String) => Unit,
-   sentinelCount: Int = 1): Unit = {
+   sentinelCount: Int = 1,
+   concurrent: Boolean = false): Unit = {
 withClue("expected number of stdout sentinels") {
   sentinelCount shouldBe StringUtils.countMatches(out, sentinel)
 }
+//sentinels should be all together
+if (concurrent) {
 
 Review comment:
   could you just count the number of sentinel occurrences to match the number 
of activations?


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


With regards,
Apache Git Services

[GitHub] rabbah commented on a change in pull request #3707: enable concurrent runs on ActionContainer test utility

[GitBox]

rabbah commented on a change in pull request #3707: enable concurrent runs on 
ActionContainer test utility 
URL: 
https://github.com/apache/incubator-openwhisk/pull/3707#discussion_r193768778
 
 

 ##
 File path: tests/src/test/scala/actionContainers/ActionContainer.scala
 ##
 @@ -65,13 +67,26 @@ trait ActionProxyContainerTestUtils extends FlatSpec with 
Matchers with StreamLo
   def checkStreams(out: String,
err: String,
additionalCheck: (String, String) => Unit,
-   sentinelCount: Int = 1): Unit = {
+   sentinelCount: Int = 1,
+   concurrent: Boolean = false): Unit = {
 withClue("expected number of stdout sentinels") {
   sentinelCount shouldBe StringUtils.countMatches(out, sentinel)
 }
+//sentinels should be all together
+if (concurrent) {
 
 Review comment:
   this seems overly strict - it will mean the runtimes have to guarantee not 
to interleave sentinels as activations finish; is that the intent?


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


With regards,
Apache Git Services

[GitHub] rabbah commented on a change in pull request #3707: enable concurrent runs on ActionContainer test utility

[GitBox]

rabbah commented on a change in pull request #3707: enable concurrent runs on 
ActionContainer test utility 
URL: 
https://github.com/apache/incubator-openwhisk/pull/3707#discussion_r193768311
 
 

 ##
 File path: common/scala/src/main/scala/whisk/core/containerpool/HttpUtils.scala
 ##
 @@ -53,10 +58,12 @@ import whisk.core.entity.size.SizeLong
  * @param hostname the host name
  * @param timeout the timeout in msecs to wait for a response
  * @param maxResponse the maximum size in bytes the connection will accept
+ * @param maxConcurrent the maximum number of concurrent requests allowed
  */
-protected[core] class HttpUtils(hostname: String, timeout: FiniteDuration, 
maxResponse: ByteSize)(
+protected[core] class HttpUtils(hostname: String, timeout: FiniteDuration, 
maxResponse: ByteSize, maxConcurrent: Int)(
 
 Review comment:
   you could do:
   
   ```
   -protected[core] class HttpUtils(hostname: String, timeout: FiniteDuration, 
maxResponse: ByteSize, maxConcurrent: Int)(
   -  implicit logging: Logging) {
   -  def this(hostname: String, timeout: FiniteDuration, maxResponse: 
ByteSize)(implicit logging: Logging) =
   -this(hostname: String, timeout: FiniteDuration, maxResponse: ByteSize, 
1)
   +protected[core] class HttpUtils(hostname: String,
   +timeout: FiniteDuration,
   +maxResponse: ByteSize,
   +maxConcurrent: Int = 1)(implicit logging: 
Logging) {
   +
   ```
   


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


With regards,
Apache Git Services

[GitHub] rabbah commented on a change in pull request #3707: enable concurrent runs on ActionContainer test utility

[GitBox]

rabbah commented on a change in pull request #3707: enable concurrent runs on 
ActionContainer test utility 
URL: 
https://github.com/apache/incubator-openwhisk/pull/3707#discussion_r190926261
 
 

 ##
 File path: common/scala/src/main/scala/whisk/core/containerpool/HttpUtils.scala
 ##
 @@ -76,11 +83,12 @@ protected[core] class HttpUtils(hostname: String, timeout: 
FiniteDuration, maxRe
 request.addHeader(HttpHeaders.ACCEPT, "application/json")
 request.setEntity(entity)
 
-execute(request, timeout.toMillis.toInt, retry)
+execute(request, timeout.toMillis.toInt, maxConcurrent, retry)
   }
 
   private def execute(request: HttpRequestBase,
   timeoutMsec: Integer,
+  maxConcurrent: Int,
 
 Review comment:
   how is `maxConcurrent` used in `execute`?


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


With regards,
Apache Git Services