[GitHub] style95 commented on a change in pull request #3671: Activation id in header
style95 commented on a change in pull request #3671: Activation id in header
URL:
https://github.com/apache/incubator-openwhisk/pull/3671#discussion_r210958917
##
File path: core/controller/src/main/scala/whisk/core/controller/Entities.scala
##
@@ -65,15 +65,24 @@ protected[controller] trait ValidateRequestSize extends
Directives {
protected val fieldDescriptionForSizeError = "Request"
}
+protected trait CustomHeaders extends Directives {
+ val ActivationIdHeader = "x-openwhisk-activation-id"
Review comment:
@rabbah Agree.
How about using x-openwhisk?
Since x- header is a custom header specific to the application, it would be
better to use the detailed one.
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] style95 commented on a change in pull request #3671: Activation id in header
style95 commented on a change in pull request #3671: Activation id in header URL: https://github.com/apache/incubator-openwhisk/pull/3671#discussion_r210770953 ## File path: tests/src/test/scala/whisk/core/controller/test/ControllerTestCommon.scala ## @@ -89,6 +89,8 @@ protected trait ControllerTestCommon val logStore = SpiLoader.get[LogStoreProvider].instance(actorSystem) val activationStore = SpiLoader.get[ActivationStoreProvider].instance(actorSystem, materializer, logging) + val ActivationIdHeaderInLowercase = ActivationIdHeader.toLowerCase + Review comment: The value in `CustomHeaders`? 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] style95 commented on a change in pull request #3671: Activation id in header
style95 commented on a change in pull request #3671: Activation id in header URL: https://github.com/apache/incubator-openwhisk/pull/3671#discussion_r210770953 ## File path: tests/src/test/scala/whisk/core/controller/test/ControllerTestCommon.scala ## @@ -89,6 +89,8 @@ protected trait ControllerTestCommon val logStore = SpiLoader.get[LogStoreProvider].instance(actorSystem) val activationStore = SpiLoader.get[ActivationStoreProvider].instance(actorSystem, materializer, logging) + val ActivationIdHeaderInLowercase = ActivationIdHeader.toLowerCase + Review comment: The value in ‘CustomHeaders’? 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] style95 commented on a change in pull request #3671: Activation id in header
style95 commented on a change in pull request #3671: Activation id in header URL: https://github.com/apache/incubator-openwhisk/pull/3671#discussion_r210619710 ## File path: tests/src/test/scala/whisk/core/controller/test/ControllerTestCommon.scala ## @@ -89,6 +89,8 @@ protected trait ControllerTestCommon val logStore = SpiLoader.get[LogStoreProvider].instance(actorSystem) val activationStore = SpiLoader.get[ActivationStoreProvider].instance(actorSystem, materializer, logging) + val ActivationIdHeaderInLowercase = ActivationIdHeader.toLowerCase + Review comment: Sorry, what is the global setting? 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] style95 commented on a change in pull request #3671: Activation id in header
style95 commented on a change in pull request #3671: Activation id in header URL: https://github.com/apache/incubator-openwhisk/pull/3671#discussion_r210619710 ## File path: tests/src/test/scala/whisk/core/controller/test/ControllerTestCommon.scala ## @@ -89,6 +89,8 @@ protected trait ControllerTestCommon val logStore = SpiLoader.get[LogStoreProvider].instance(actorSystem) val activationStore = SpiLoader.get[ActivationStoreProvider].instance(actorSystem, materializer, logging) + val ActivationIdHeaderInLowercase = ActivationIdHeader.toLowerCase + Review comment: Sorry, what is global setting? 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] style95 commented on a change in pull request #3671: Activation id in header
style95 commented on a change in pull request #3671: Activation id in header URL: https://github.com/apache/incubator-openwhisk/pull/3671#discussion_r210619710 ## File path: tests/src/test/scala/whisk/core/controller/test/ControllerTestCommon.scala ## @@ -89,6 +89,8 @@ protected trait ControllerTestCommon val logStore = SpiLoader.get[LogStoreProvider].instance(actorSystem) val activationStore = SpiLoader.get[ActivationStoreProvider].instance(actorSystem, materializer, logging) + val ActivationIdHeaderInLowercase = ActivationIdHeader.toLowerCase + Review comment: Sorry, what does global setting mean? 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] style95 commented on a change in pull request #3671: Activation id in header
style95 commented on a change in pull request #3671: Activation id in header
URL:
https://github.com/apache/incubator-openwhisk/pull/3671#discussion_r207696273
##
File path:
tests/src/test/scala/whisk/core/controller/test/WebActionsApiTests.scala
##
@@ -932,7 +934,10 @@ trait WebActionsApiBaseTests extends ControllerTestCommon
with BeforeAndAfterEac
m(s"$testRoutePath/$path") ~> Route.seal(routes(creds)) ~> check {
status should be(statusCode)
- headers shouldBe List(RawHeader("Set-Cookie", "a=b"))
+ headers.exists(header => {
+header.is("set-cookie") && header.value() == "a=b"
+ }) shouldBe true
+ headers.exists(_.is(ActivationIdHeaderInLowercase)) shouldBe true
Review comment:
got it
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] style95 commented on a change in pull request #3671: Activation id in header
style95 commented on a change in pull request #3671: Activation id in header URL: https://github.com/apache/incubator-openwhisk/pull/3671#discussion_r190837409 ## File path: tests/src/test/scala/whisk/core/controller/test/ControllerTestCommon.scala ## @@ -91,6 +91,8 @@ protected trait ControllerTestCommon val authStore = WhiskAuthStore.datastore() val logStore = SpiLoader.get[LogStoreProvider].logStore(actorSystem) + val ActivationIdHeaderInLowercase = "x-openwhisk-activation-id" Review comment: Updated accordingly. Regarding case, I followed scala constant naming convention(Upper camel case). Is it OW convention(start with a lowercase letter)? 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] style95 commented on a change in pull request #3671: Activation id in header
style95 commented on a change in pull request #3671: Activation id in header
URL:
https://github.com/apache/incubator-openwhisk/pull/3671#discussion_r190836887
##
File path: core/controller/src/main/scala/whisk/core/controller/Entities.scala
##
@@ -65,14 +65,25 @@ protected[controller] trait ValidateRequestSize extends
Directives {
protected val fieldDescriptionForSizeError = "Request"
}
+protected trait CustomHeaders extends Directives {
+
+ /** Add activation ID in headers */
+ protected def respondWithActivationIdHeader(activation: WhiskActivation):
Directive0 =
Review comment:
Updated accordingly
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] style95 commented on a change in pull request #3671: Activation id in header
style95 commented on a change in pull request #3671: Activation id in header
URL:
https://github.com/apache/incubator-openwhisk/pull/3671#discussion_r190531314
##
File path: core/controller/src/main/scala/whisk/core/controller/Actions.scala
##
@@ -257,24 +256,31 @@ trait WhiskActionsApi extends WhiskCollectionAPI with
PostActionActivation with
onComplete(invokeAction(user, actionWithMergedParams, payload,
waitForResponse, cause = None)) {
case Success(Left(activationId)) =>
// non-blocking invoke or blocking invoke which got queued instead
-complete(Accepted, activationId.toJsObject)
+
+respondWithActivationIdHeader(activationId) {
+ // note that if header defined a content-type, it will be ignored
+ // since the type must be compatible with the data response
Review comment:
removed.
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] style95 commented on a change in pull request #3671: Activation id in header
style95 commented on a change in pull request #3671: Activation id in header
URL:
https://github.com/apache/incubator-openwhisk/pull/3671#discussion_r190526404
##
File path:
core/controller/src/main/scala/whisk/core/controller/WebActions.scala
##
@@ -634,37 +634,40 @@ trait WhiskWebActionsApi extends Directives with
ValidateRequestSize with PostAc
responseType: MediaExtension)(implicit transid:
TransactionId) = {
onComplete(queuedActivation) {
case Success(Right(activation)) =>
-val result = activation.resultAsJson
+respondWithActivationIdHeader(activation) {
Review comment:
This is to cope with the inconvenience which is described in #3582.
This will give users a better option to retrieve right activation results.
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
