[GitHub] [openwhisk] style95 commented on a diff in pull request #5229: Provide action limit configuration for each namespace

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

style95 commented on code in PR #5229:
URL: https://github.com/apache/openwhisk/pull/5229#discussion_r897866510


##########
common/scala/src/main/scala/org/apache/openwhisk/core/containerpool/ApacheBlockingContainerClient.scala:
##########
@@ -124,13 +129,13 @@ protected class ApacheBlockingContainerClient(hostname: String,
 
           // Negative contentLength means unknown or overflow. We don't want to consume in either case.
           if (contentLength >= 0) {
-            if (contentLength <= maxResponseBytes) {
+            if (contentLength <= maxResponse.toBytes) {

Review Comment:
   Do we need `toBytes` here?



##########
common/scala/src/main/scala/org/apache/openwhisk/core/containerpool/AkkaContainerClient.scala:
##########
@@ -87,12 +85,18 @@ protected class AkkaContainerClient(
    *
    * @param endpoint the path the api call relative to hostname
    * @param body the JSON value to post (this is usually a JSON objecT)
+   * @param maxResponse the maximum size in bytes the connection will accept

Review Comment:
   Seems the description about the `truncation` is missing.
   



##########
common/scala/src/main/scala/org/apache/openwhisk/core/entity/ActivationEntityLimit.scala:
##########
@@ -32,8 +32,20 @@ case class ActivationEntityLimitConf(serdesOverhead: ByteSize, payload: Activati
  */
 protected[core] object ActivationEntityLimit {
   private val config = loadConfigOrThrow[ActivationEntityLimitConf](ConfigKeys.activation)
+  private val namespacePayloadLimitConfig = try {
+    loadConfigOrThrow[ActivationEntityPayload](ConfigKeys.namespaceActivationPayloadLimit)
+  } catch {
+    case _: Throwable =>
+      // Supports backwards compatibility for openwhisk that do not use the namespace default limit
+      ActivationEntityPayload(config.payload.max, config.payload.truncation)

Review Comment:
   👍 



##########
tests/src/test/scala/org/apache/openwhisk/core/entity/test/SchemaTests.scala:
##########
@@ -826,61 +825,16 @@ class SchemaTests extends FlatSpec with BeforeAndAfter with ExecHelpers with Mat
 
     serdes foreach { s =>
       withClue(s"serializer $s") {
-        if (s != LogLimit.serdes) {
-          val lb = the[DeserializationException] thrownBy s.read(JsNumber(0))
-          lb.getMessage should include("below allowed threshold")
-        } else {
+        if (s == LogLimit.serdes) {
           val lb = the[DeserializationException] thrownBy s.read(JsNumber(-1))
           lb.getMessage should include("a negative size of an object is not allowed")
         }
-
-        val ub = the[DeserializationException] thrownBy s.read(JsNumber(Int.MaxValue))
-        ub.getMessage should include("exceeds allowed threshold")
-
         val int = the[DeserializationException] thrownBy s.read(JsNumber(2.5))
         int.getMessage should include("limit must be whole number")
       }
     }
   }
 
-  it should "reject bad limit values" in {

Review Comment:
   Don't we need this negative test case?



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org