[GitHub] [openwhisk] codecov-io edited a comment on issue #4628: Embedded Kafka support in OpenWhisk Standalone mode
codecov-io edited a comment on issue #4628: Embedded Kafka support in OpenWhisk Standalone mode URL: https://github.com/apache/openwhisk/pull/4628#issuecomment-53154 # [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4628?src=pr&el=h1) Report > Merging [#4628](https://codecov.io/gh/apache/openwhisk/pull/4628?src=pr&el=desc) into [master](https://codecov.io/gh/apache/openwhisk/commit/06362ad83608335a01ccbb4ef449ec4c0e70dd99?src=pr&el=desc) will **increase** coverage by `35.54%`. > The diff coverage is `n/a`. [![Impacted file tree graph](https://codecov.io/gh/apache/openwhisk/pull/4628/graphs/tree.svg?width=650&token=l0YmsiSAso&height=150&src=pr)](https://codecov.io/gh/apache/openwhisk/pull/4628?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master#4628 +/- ## === + Coverage 43.31% 78.85% +35.54% === Files 183 183 Lines8305 8305 Branches 574 574 === + Hits 3597 6549 +2952 + Misses 4708 1756 -2952 ``` | [Impacted Files](https://codecov.io/gh/apache/openwhisk/pull/4628?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [...openwhisk/common/tracing/OpenTracingProvider.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvbW1vbi90cmFjaW5nL09wZW5UcmFjaW5nUHJvdmlkZXIuc2NhbGE=) | `21.15% <0%> (+1.92%)` | :arrow_up: | | [...re/database/MultipleReadersSingleWriterCache.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvTXVsdGlwbGVSZWFkZXJzU2luZ2xlV3JpdGVyQ2FjaGUuc2NhbGE=) | `98% <0%> (+2%)` | :arrow_up: | | [...apache/openwhisk/core/entitlement/Collection.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXRsZW1lbnQvQ29sbGVjdGlvbi5zY2FsYQ==) | `87.5% <0%> (+2.5%)` | :arrow_up: | | [.../org/apache/openwhisk/http/PoolingRestClient.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2h0dHAvUG9vbGluZ1Jlc3RDbGllbnQuc2NhbGE=) | `91.17% <0%> (+2.94%)` | :arrow_up: | | [...abase/cosmosdb/CosmosDBArtifactStoreProvider.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlUHJvdmlkZXIuc2NhbGE=) | `4% <0%> (+4%)` | :arrow_up: | | [...tainerpool/docker/DockerClientWithFileAccess.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udGFpbmVycG9vbC9kb2NrZXIvRG9ja2VyQ2xpZW50V2l0aEZpbGVBY2Nlc3Muc2NhbGE=) | `96% <0%> (+4%)` | :arrow_up: | | [...whisk/connector/kafka/KafkaConsumerConnector.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2Nvbm5lY3Rvci9rYWZrYS9LYWZrYUNvbnN1bWVyQ29ubmVjdG9yLnNjYWxh) | `57.74% <0%> (+4.22%)` | :arrow_up: | | [...enwhisk/core/loadBalancer/InvokerSupervision.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvbG9hZEJhbGFuY2VyL0ludm9rZXJTdXBlcnZpc2lvbi5zY2FsYQ==) | `95.77% <0%> (+4.92%)` | :arrow_up: | | [...pache/openwhisk/core/entity/ConcurrencyLimit.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXR5L0NvbmN1cnJlbmN5TGltaXQuc2NhbGE=) | `94.11% <0%> (+5.88%)` | :arrow_up: | | [...la/org/apache/openwhisk/core/entity/LogLimit.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXR5L0xvZ0xpbWl0LnNjYWxh) | `94.11% <0%> (+5.88%)` | :arrow_up: | | ... and [123 more](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/openwhisk/pull/4628?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4628?src=pr&el=footer). Last update [06362ad...4d45b09](https://codecov.io/gh/apache/openwhisk/pull/4628?src=
[GitHub] [openwhisk] sven-lange-last commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families
sven-lange-last commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families URL: https://github.com/apache/openwhisk/pull/4627#discussion_r325541841 ## File path: docs/actions-update.md ## @@ -0,0 +1,75 @@ + +## Updating Action Language Runtimes + +OpenWhisk supports [several languages and runtimes](actions.md#languages-and-runtimes) that can be made +available for usage in an OpenWhisk deployment. This is done via the [runtimes manifest](actions-new.md#the-runtimes-manifest). + +Over time, you may have the need for change: + +* Update runtimes to address security issues - for example, the latest code level of Node.js 10. +* Remove runtime versions that are no more supported - for example, Node.js 6. +* Add more languages due to user demand. +* Remove languages that are no more needed. + +While adding and updating languages and runtimes is pretty straightforward, removing or renaming languages and runtimes +requires some caution to prevent problems with existing actions. + +### Updating runtimes + +Follow these steps to update a particular runtime kind: + +1. Update the runtimes' container image. +2. Update the corresponding `image` property in the [runtimes manifest](actions-new.md#the-runtimes-manifest) to use the new container image. +3. Restart / re-deploy controllers and invokers such that they pick up the changed runtimes manifest. + +Already existing actions of the changed runtime kind will immediately use the new container image when the invoker creates a new action container. + +Apparently, this approach should only be used if existing actions do not break with the new container image. If a new container image is supposed to break existing actions, introduce a new runtime kind. + +### Removing runtimes + +Follow these steps to remove a particular runtime kind under the assumption that actions with the runtime kind exist in the system. Apparently, it makes sense to wait some time between accomplishing these steps to give action owners time to react. + +1. Deprecate the runtime kind by setting `"deprecated": true` in the [runtimes manifest](actions-new.md#the-runtimes-manifest). This setting prevents that new actions can be created with the deprecated action kind. In addition, existing actions cannot be changed to the deprecated action kind any more. +2. Ask owners of existing actions with runtime kind to be removed to update their actions to a different action kind. +3. Create an automated process that identifies all actions with the runtime kind to be removed in the system's action artifact store. Either automatically remove these actions or change to a different runtime kind. +4. Once the system's action artifact store does no more contain actions with the runtime kind to be removed, remove the runtime kind from the [runtimes manifest](actions-new.md#the-runtimes-manifest). +5. TODO - is this correct?: Remove the runtime kind from the list of known kinds in the `ActionExec` object of the [controller API's Swagger definition](../core/controller/src/main/resources/apiv1swagger.json). Review comment: Actually, I wonder whether a fixed list of supported runtime kinds in the Swagger API definition makes sense at all... * We expect that each adopter will have their own set of supported runtime families and kinds resulting in a customised version of the `runtimes.json` file. * We have a mechanism in Openwhisk that allows adopters to configure their own `runtimes.json` instead of using the default file: https://github.com/apache/openwhisk/blob/06362ad83608335a01ccbb4ef449ec4c0e70dd99/ansible/group_vars/all#L54-L69. * In order to obtain a sound Swagger API definition, each adopter has to synchronise [resource file `apiv1swagger.json`](../core/controller/src/main/resources/apiv1swagger.json). Maybe I'm missing something - but I see no configuration mechanism to replace the resource file provided by Openwhisk. So replacing this file requires some extra operations... * At the same time, the controller provides a REST endpoint where it publishes all currently supported runtime families and kinds. This information comes from the configured `runtimes.json` file. * Wouldn't it make sense to remove the runtime kind `enum` from the Swagger API definition (https://github.com/apache/openwhisk/blob/06362ad83608335a01ccbb4ef449ec4c0e70dd99/core/controller/src/main/resources/apiv1swagger.json#L1890-L1915) and add a hint to the description that there is a REST endpoint where the current list can be obtained? Does anybody know whether the `enum` data is used somewhere? 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. For queries about this service, please contact Infrastructure
[GitHub] [openwhisk] sven-lange-last commented on a change in pull request #4624: Combines active ack and slot release when both are available.
sven-lange-last commented on a change in pull request #4624: Combines active ack and slot release when both are available. URL: https://github.com/apache/openwhisk/pull/4624#discussion_r325546418 ## File path: common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala ## @@ -64,43 +64,34 @@ case class ActivationMessage(override val transid: TransactionId, def causedBySequence: Boolean = cause.isDefined } -object ActivationMessage extends DefaultJsonProtocol { - - def parse(msg: String) = Try(serdes.read(msg.parseJson)) - - private implicit val fqnSerdes = FullyQualifiedEntityName.serdes - implicit val serdes = jsonFormat11(ActivationMessage.apply) -} - /** * Message that is sent from the invoker to the controller after action is completed or after slot is free again for * new actions. */ abstract class AcknowledegmentMessage(private val tid: TransactionId) extends Message { override val transid: TransactionId = tid - override def serialize: String = { -AcknowledegmentMessage.serdes.write(this).compactPrint - } + override def serialize: String = AcknowledegmentMessage.serdes.write(this).compactPrint + def toJson: JsValue } /** - * This message is sent from the invoker to the controller, after the slot of an invoker that has been used by the - * current action, is free again (after log collection) + * This message is sent from an invoker to the controller, after the resource slot in the invoke, used by the + * corresponding activation, is free again (i.e., after log collection). In some cases, the activation result is + * ready and the slot is freed at the same time. In such cases, the completion message carries the result as well. + * This is reflected by the of a Right() `response` and the param `result` is set to true. + * In some cases the `result` is true but the response is Left() if the message was too large for the message bus. */ case class CompletionMessage(override val transid: TransactionId, - activationId: ActivationId, - isSystemError: Boolean, + response: Either[ActivationId, WhiskActivation], + result: Boolean, // true iff the message is a combined active ack and slot released Review comment: @rabbah thanks for your openness to adjust your work to our feedback. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] codecov-io commented on issue #4631: Update CosmosDB SDK to v2.5.0
codecov-io commented on issue #4631: Update CosmosDB SDK to v2.5.0 URL: https://github.com/apache/openwhisk/pull/4631#issuecomment-532582996 # [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4631?src=pr&el=h1) Report > Merging [#4631](https://codecov.io/gh/apache/openwhisk/pull/4631?src=pr&el=desc) into [master](https://codecov.io/gh/apache/openwhisk/commit/06362ad83608335a01ccbb4ef449ec4c0e70dd99?src=pr&el=desc) will **increase** coverage by `35.55%`. > The diff coverage is `n/a`. [![Impacted file tree graph](https://codecov.io/gh/apache/openwhisk/pull/4631/graphs/tree.svg?width=650&token=l0YmsiSAso&height=150&src=pr)](https://codecov.io/gh/apache/openwhisk/pull/4631?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master#4631 +/- ## === + Coverage 43.31% 78.86% +35.55% === Files 183 183 Lines8305 8305 Branches 574 574 === + Hits 3597 6550 +2953 + Misses 4708 1755 -2953 ``` | [Impacted Files](https://codecov.io/gh/apache/openwhisk/pull/4631?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [...openwhisk/common/tracing/OpenTracingProvider.scala](https://codecov.io/gh/apache/openwhisk/pull/4631/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvbW1vbi90cmFjaW5nL09wZW5UcmFjaW5nUHJvdmlkZXIuc2NhbGE=) | `21.15% <0%> (+1.92%)` | :arrow_up: | | [...re/database/MultipleReadersSingleWriterCache.scala](https://codecov.io/gh/apache/openwhisk/pull/4631/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvTXVsdGlwbGVSZWFkZXJzU2luZ2xlV3JpdGVyQ2FjaGUuc2NhbGE=) | `98% <0%> (+2%)` | :arrow_up: | | [...apache/openwhisk/core/entitlement/Collection.scala](https://codecov.io/gh/apache/openwhisk/pull/4631/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXRsZW1lbnQvQ29sbGVjdGlvbi5zY2FsYQ==) | `87.5% <0%> (+2.5%)` | :arrow_up: | | [.../org/apache/openwhisk/http/PoolingRestClient.scala](https://codecov.io/gh/apache/openwhisk/pull/4631/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2h0dHAvUG9vbGluZ1Jlc3RDbGllbnQuc2NhbGE=) | `91.17% <0%> (+2.94%)` | :arrow_up: | | [...abase/cosmosdb/CosmosDBArtifactStoreProvider.scala](https://codecov.io/gh/apache/openwhisk/pull/4631/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlUHJvdmlkZXIuc2NhbGE=) | `4% <0%> (+4%)` | :arrow_up: | | [...tainerpool/docker/DockerClientWithFileAccess.scala](https://codecov.io/gh/apache/openwhisk/pull/4631/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udGFpbmVycG9vbC9kb2NrZXIvRG9ja2VyQ2xpZW50V2l0aEZpbGVBY2Nlc3Muc2NhbGE=) | `96% <0%> (+4%)` | :arrow_up: | | [...whisk/connector/kafka/KafkaConsumerConnector.scala](https://codecov.io/gh/apache/openwhisk/pull/4631/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2Nvbm5lY3Rvci9rYWZrYS9LYWZrYUNvbnN1bWVyQ29ubmVjdG9yLnNjYWxh) | `57.74% <0%> (+4.22%)` | :arrow_up: | | [...enwhisk/core/loadBalancer/InvokerSupervision.scala](https://codecov.io/gh/apache/openwhisk/pull/4631/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvbG9hZEJhbGFuY2VyL0ludm9rZXJTdXBlcnZpc2lvbi5zY2FsYQ==) | `95.77% <0%> (+4.92%)` | :arrow_up: | | [...pache/openwhisk/core/entity/ConcurrencyLimit.scala](https://codecov.io/gh/apache/openwhisk/pull/4631/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXR5L0NvbmN1cnJlbmN5TGltaXQuc2NhbGE=) | `94.11% <0%> (+5.88%)` | :arrow_up: | | [...la/org/apache/openwhisk/core/entity/LogLimit.scala](https://codecov.io/gh/apache/openwhisk/pull/4631/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXR5L0xvZ0xpbWl0LnNjYWxh) | `94.11% <0%> (+5.88%)` | :arrow_up: | | ... and [123 more](https://codecov.io/gh/apache/openwhisk/pull/4631/diff?src=pr&el=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/openwhisk/pull/4631?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4631?src=pr&el=footer). Last update [06362ad...2bcd965](https://codecov.io/gh/apache/openwhisk/pull/4631?src=pr&el=lastupdated). Read the
[GitHub] [openwhisk] falkzoll commented on issue #4627: Add descriptions how to update, remove or rename runtime kinds and language families
falkzoll commented on issue #4627: Add descriptions how to update, remove or rename runtime kinds and language families URL: https://github.com/apache/openwhisk/pull/4627#issuecomment-532582932 👍 to document how to remove an existing runtime/kind in a controlled way from an already running openwhisk makes a lot of sense. Thanks for providing this guidance. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] BillZong commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families
BillZong commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families URL: https://github.com/apache/openwhisk/pull/4627#discussion_r325558459 ## File path: docs/actions-update.md ## @@ -0,0 +1,75 @@ + +## Updating Action Language Runtimes + +OpenWhisk supports [several languages and runtimes](actions.md#languages-and-runtimes) that can be made +available for usage in an OpenWhisk deployment. This is done via the [runtimes manifest](actions-new.md#the-runtimes-manifest). + +Over time, you may have the need for change: + +* Update runtimes to address security issues - for example, the latest code level of Node.js 10. +* Remove runtime versions that are no more supported - for example, Node.js 6. +* Add more languages due to user demand. +* Remove languages that are no more needed. + +While adding and updating languages and runtimes is pretty straightforward, removing or renaming languages and runtimes +requires some caution to prevent problems with existing actions. + +### Updating runtimes + +Follow these steps to update a particular runtime kind: + +1. Update the runtimes' container image. +2. Update the corresponding `image` property in the [runtimes manifest](actions-new.md#the-runtimes-manifest) to use the new container image. +3. Restart / re-deploy controllers and invokers such that they pick up the changed runtimes manifest. + +Already existing actions of the changed runtime kind will immediately use the new container image when the invoker creates a new action container. + +Apparently, this approach should only be used if existing actions do not break with the new container image. If a new container image is supposed to break existing actions, introduce a new runtime kind. + +### Removing runtimes + +Follow these steps to remove a particular runtime kind under the assumption that actions with the runtime kind exist in the system. Apparently, it makes sense to wait some time between accomplishing these steps to give action owners time to react. + +1. Deprecate the runtime kind by setting `"deprecated": true` in the [runtimes manifest](actions-new.md#the-runtimes-manifest). This setting prevents that new actions can be created with the deprecated action kind. In addition, existing actions cannot be changed to the deprecated action kind any more. +2. Ask owners of existing actions with runtime kind to be removed to update their actions to a different action kind. +3. Create an automated process that identifies all actions with the runtime kind to be removed in the system's action artifact store. Either automatically remove these actions or change to a different runtime kind. +4. Once the system's action artifact store does no more contain actions with the runtime kind to be removed, remove the runtime kind from the [runtimes manifest](actions-new.md#the-runtimes-manifest). +5. TODO - is this correct?: Remove the runtime kind from the list of known kinds in the `ActionExec` object of the [controller API's Swagger definition](../core/controller/src/main/resources/apiv1swagger.json). Review comment: Don't get any idea about it. Remove it and run the tests would make sense. [Local development](https://github.com/apache/openwhisk/tree/master/ansible/README.md) and [Unit Test](https://github.com/apache/openwhisk/tree/master/tests/README.md) 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] falkzoll commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families
falkzoll commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families URL: https://github.com/apache/openwhisk/pull/4627#discussion_r325561251 ## File path: docs/actions-update.md ## @@ -0,0 +1,75 @@ + +## Updating Action Language Runtimes + +OpenWhisk supports [several languages and runtimes](actions.md#languages-and-runtimes) that can be made +available for usage in an OpenWhisk deployment. This is done via the [runtimes manifest](actions-new.md#the-runtimes-manifest). + +Over time, you may have the need for change: + +* Update runtimes to address security issues - for example, the latest code level of Node.js 10. +* Remove runtime versions that are no more supported - for example, Node.js 6. +* Add more languages due to user demand. +* Remove languages that are no more needed. + +While adding and updating languages and runtimes is pretty straightforward, removing or renaming languages and runtimes +requires some caution to prevent problems with existing actions. + +### Updating runtimes + +Follow these steps to update a particular runtime kind: + +1. Update the runtimes' container image. +2. Update the corresponding `image` property in the [runtimes manifest](actions-new.md#the-runtimes-manifest) to use the new container image. +3. Restart / re-deploy controllers and invokers such that they pick up the changed runtimes manifest. + +Already existing actions of the changed runtime kind will immediately use the new container image when the invoker creates a new action container. + +Apparently, this approach should only be used if existing actions do not break with the new container image. If a new container image is supposed to break existing actions, introduce a new runtime kind. + +### Removing runtimes + +Follow these steps to remove a particular runtime kind under the assumption that actions with the runtime kind exist in the system. Apparently, it makes sense to wait some time between accomplishing these steps to give action owners time to react. + +1. Deprecate the runtime kind by setting `"deprecated": true` in the [runtimes manifest](actions-new.md#the-runtimes-manifest). This setting prevents that new actions can be created with the deprecated action kind. In addition, existing actions cannot be changed to the deprecated action kind any more. +2. Ask owners of existing actions with runtime kind to be removed to update their actions to a different action kind. +3. Create an automated process that identifies all actions with the runtime kind to be removed in the system's action artifact store. Either automatically remove these actions or change to a different runtime kind. +4. Once the system's action artifact store does no more contain actions with the runtime kind to be removed, remove the runtime kind from the [runtimes manifest](actions-new.md#the-runtimes-manifest). +5. TODO - is this correct?: Remove the runtime kind from the list of known kinds in the `ActionExec` object of the [controller API's Swagger definition](../core/controller/src/main/resources/apiv1swagger.json). Review comment: I would support to remove this enum from the swagger documentation and having a pointer to the REST endpoint instead. We may think about mentioning two or three samples of stable/longer time supported runtimes, e.g. nodejs:12, nodejs:default, python:3 in this description, just that users get a feeling how the values can look like, but do not specify the whole list. I have not seen unit tests, that rely on this swagger enum, yet, but in case they exist, I would prefer that we change them to query the REST endpoint, instead of querying the info from the swagger documentation. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] sven-lange-last commented on a change in pull request #4503: Add optional config for appending custom registry to user provided images
sven-lange-last commented on a change in pull request #4503: Add optional config for appending custom registry to user provided images URL: https://github.com/apache/openwhisk/pull/4503#discussion_r325577206 ## File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/docker/DockerContainerFactory.scala ## @@ -60,10 +62,14 @@ class DockerContainerFactory(instance: InvokerInstanceId, userProvidedImage: Boolean, memory: ByteSize, cpuShares: Int)(implicit config: WhiskConfig, logging: Logging): Future[Container] = { +val registryConfig = + ContainerFactory.resolveRegisterConfig(userProvidedImage, runtimesRegistryConfig, userImagesRegistryConfig) +val image = + if (userProvidedImage && registryConfig.url.isEmpty) Left(actionImage) + else Right(actionImage.localImageName(registryConfig.url)) Review comment: My understanding is that the following behaviour is desired for blackbox (i.e. user provided) images identified by a user provided image reference (see https://github.com/docker/distribution/blob/master/reference/reference.go): * If the image reference contains a domain, keep the image reference as is to pull from the repository in the image reference. * If the image reference contains NO domain and NO global user image registry is configured (`registryConfig.url.isEmpty == true`), keep the image reference as is. It's up to the used container factory to convert to a canonical reference including a domain and path. * If the image reference contains NO domain and a global user image registry is configured (`registryConfig.url.isEmpty == false`), add the global user image registry as domain to the image reference. I think this is exactly the behaviour that following code implements: ``` val registryConfig = ContainerFactory.resolveRegisterConfig(userProvidedImage, runtimesRegistryConfig, userImagesRegistryConfig) val imageRef = ContainerFactory.resolveImage(actionImage, registryConfig) ``` That's why I proposed this code because it already addresses the three cases listed above. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] duynguyen commented on a change in pull request #4503: Add optional config for appending custom registry to user provided images
duynguyen commented on a change in pull request #4503: Add optional config for appending custom registry to user provided images URL: https://github.com/apache/openwhisk/pull/4503#discussion_r325605361 ## File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/docker/DockerContainerFactory.scala ## @@ -60,10 +62,14 @@ class DockerContainerFactory(instance: InvokerInstanceId, userProvidedImage: Boolean, memory: ByteSize, cpuShares: Int)(implicit config: WhiskConfig, logging: Logging): Future[Container] = { +val registryConfig = + ContainerFactory.resolveRegisterConfig(userProvidedImage, runtimesRegistryConfig, userImagesRegistryConfig) +val image = + if (userProvidedImage && registryConfig.url.isEmpty) Left(actionImage) + else Right(actionImage.localImageName(registryConfig.url)) Review comment: alright, now I got what you meant, thanks for explaining. My intention with "If the image reference contains a domain", is that if there is a global user image registry, it would overwrite the image domain. I thought of this in a multi-cloud / multi-region setup in which a cluster for a specific cloud / region could force to download *all* images from its desired registry. But I also agree with you that keeping the domain in image reference (if existed) no matter what global registry is, would be more trivial to users. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] sven-lange-last commented on a change in pull request #4503: Add optional config for appending custom registry to user provided images
sven-lange-last commented on a change in pull request #4503: Add optional config for appending custom registry to user provided images URL: https://github.com/apache/openwhisk/pull/4503#discussion_r325625604 ## File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/docker/DockerContainerFactory.scala ## @@ -60,10 +62,14 @@ class DockerContainerFactory(instance: InvokerInstanceId, userProvidedImage: Boolean, memory: ByteSize, cpuShares: Int)(implicit config: WhiskConfig, logging: Logging): Future[Container] = { +val registryConfig = + ContainerFactory.resolveRegisterConfig(userProvidedImage, runtimesRegistryConfig, userImagesRegistryConfig) +val image = + if (userProvidedImage && registryConfig.url.isEmpty) Left(actionImage) + else Right(actionImage.localImageName(registryConfig.url)) Review comment: Yes, since your PR does not provide a list with mappings for registry domains but only a single user image repository domain, all user provided image refs would be overwritten - regardless whether the user provided image ref already contains a domain or not. This current approach would lead to: * User registry config empty, user-provided image ref `image:latest` => Docker container factory pulls `docker.io/library/image:latest`. * User registry config empty, user-provided image ref `docker.io/library/image:latest` => Docker container factory pulls `docker.io/library/image:latest`. * User registry config `uk.icr.io`, user-provided image ref `image:latest` => Docker container factory pulls `uk.icr.io/image:latest`. The user would probably expect to pull `uk.icr.io/library/image:latest` instead - but `docker pull` only inserts the "official" repository path `library` if `docker.io` is specified as domain. See https://github.com/docker/distribution/blob/14b96e55d84c367ebab6caf9f0086de0876eec72/reference/normalize.go#L88-L105. * User registry config `uk.icr.io`, user-provided image ref `path/image:latest` => Docker container factory pulls `uk.icr.io/path/image:latest`. * User registry config `uk.icr.io`, user-provided image ref `docker.io/path/image:latest` => Docker container factory pulls `uk.icr.io/path/image:latest`. The user would certainly not expect this outcome. If you provide an image ref with a domain, you would not expect that it is replaced. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] sven-lange-last commented on a change in pull request #4503: Add optional config for appending custom registry to user provided images
sven-lange-last commented on a change in pull request #4503: Add optional config for appending custom registry to user provided images URL: https://github.com/apache/openwhisk/pull/4503#discussion_r325625604 ## File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/docker/DockerContainerFactory.scala ## @@ -60,10 +62,14 @@ class DockerContainerFactory(instance: InvokerInstanceId, userProvidedImage: Boolean, memory: ByteSize, cpuShares: Int)(implicit config: WhiskConfig, logging: Logging): Future[Container] = { +val registryConfig = + ContainerFactory.resolveRegisterConfig(userProvidedImage, runtimesRegistryConfig, userImagesRegistryConfig) +val image = + if (userProvidedImage && registryConfig.url.isEmpty) Left(actionImage) + else Right(actionImage.localImageName(registryConfig.url)) Review comment: Yes, since your PR does not provide a list with mappings for registry domains but only a single user image repository domain, all user provided image refs would be overwritten - regardless whether the user provided image ref already contains a domain or not. This current approach would lead to: With a user registry config like `uk.icr.io`, this would lead to situations like: * User registry config empty, user-provided image ref `image:latest` => Docker container factory pulls `docker.io/library/image:latest`. * User registry config empty, user-provided image ref `docker.io/library/image:latest` => Docker container factory pulls `docker.io/library/image:latest`. * User registry config `uk.icr.io`, user-provided image ref `image:latest` => Docker container factory pulls `uk.icr.io/image:latest`. The user would probably expect to pull `uk.icr.io/library/image:latest` instead - but `docker pull` only inserts the "official" repository path `library` if `docker.io` is specified as domain. See https://github.com/docker/distribution/blob/14b96e55d84c367ebab6caf9f0086de0876eec72/reference/normalize.go#L88-L105. * User registry config `uk.icr.io`, user-provided image ref `path/image:latest` => Docker container factory pulls `uk.icr.io/path/image:latest`. * User registry config `uk.icr.io`, user-provided image ref `docker.io/path/image:latest` => Docker container factory pulls `uk.icr.io/path/image:latest`. The user would certainly not expect this outcome. If you provide an image ref with a domain, you would not expect that it is replaced. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] chetanmeh commented on issue #4631: Update CosmosDB SDK to v2.5.0
chetanmeh commented on issue #4631: Update CosmosDB SDK to v2.5.0 URL: https://github.com/apache/openwhisk/pull/4631#issuecomment-532653589 Build passed on my [fork repo][1] which has CosmosDB related tests enabled [1]: https://travis-ci.org/chetanmeh/incubator-openwhisk/builds/586383538 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk-utilities] sven-lange-last opened a new pull request #72: Improve pre-commit sample for openwhisk
sven-lange-last opened a new pull request #72: Improve pre-commit sample for openwhisk URL: https://github.com/apache/openwhisk-utilities/pull/72 The current code sample does not scan the full openwhisk directory but only starting from the current directory. In addition, no configuration file is specified. With this change, the pre-commit hook scans the full openwhisk directory and uses the scan configuration that is also used in the openwhisk Travis configuration. https://github.com/apache/openwhisk/blob/06362ad83608335a01ccbb4ef449ec4c0e70dd99/tools/travis/scan.sh#L35-L37 The openwhisk Travis script does not consider the repo's `.gitignore` file because it always runs on a fresh clone. A pre-commit hook runs on a working clone so that build directories specified in the `.gitignore` file must be excluded. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] chetanmeh commented on a change in pull request #4624: Combines active ack and slot release when both are available.
chetanmeh commented on a change in pull request #4624: Combines active ack and slot release when both are available. URL: https://github.com/apache/openwhisk/pull/4624#discussion_r325677965 ## File path: common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala ## @@ -64,43 +64,34 @@ case class ActivationMessage(override val transid: TransactionId, def causedBySequence: Boolean = cause.isDefined } -object ActivationMessage extends DefaultJsonProtocol { - - def parse(msg: String) = Try(serdes.read(msg.parseJson)) - - private implicit val fqnSerdes = FullyQualifiedEntityName.serdes - implicit val serdes = jsonFormat11(ActivationMessage.apply) -} - /** * Message that is sent from the invoker to the controller after action is completed or after slot is free again for * new actions. */ abstract class AcknowledegmentMessage(private val tid: TransactionId) extends Message { override val transid: TransactionId = tid - override def serialize: String = { -AcknowledegmentMessage.serdes.write(this).compactPrint - } + override def serialize: String = AcknowledegmentMessage.serdes.write(this).compactPrint + def toJson: JsValue } /** - * This message is sent from the invoker to the controller, after the slot of an invoker that has been used by the - * current action, is free again (after log collection) + * This message is sent from an invoker to the controller, after the resource slot in the invoke, used by the + * corresponding activation, is free again (i.e., after log collection). In some cases, the activation result is + * ready and the slot is freed at the same time. In such cases, the completion message carries the result as well. + * This is reflected by the of a Right() `response` and the param `result` is set to true. + * In some cases the `result` is true but the response is Left() if the message was too large for the message bus. */ case class CompletionMessage(override val transid: TransactionId, - activationId: ActivationId, - isSystemError: Boolean, + response: Either[ActivationId, WhiskActivation], + result: Boolean, // true iff the message is a combined active ack and slot released Review comment: @rabbah As you change the current design I would also like to highlight a potential change I need for my activation persister service work Per my current understand the `sendActiveAck` flow is like - `isSlotFree` false - active ack/ResultMessage - blocking - `WhiskActivation` - non-blocking - NONE - `isSlotFree` true - result/CompletionMessage - blocking - `CompletionMessage` - non-blocking - `CompletionMessage` Later I would like to have a configurable support for sending `WhiskActivation` for non blocking call as well to a custom topic. Just wanted to make you aware on that 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] chetanmeh commented on a change in pull request #4624: Combines active ack and slot release when both are available.
chetanmeh commented on a change in pull request #4624: Combines active ack and slot release when both are available. URL: https://github.com/apache/openwhisk/pull/4624#discussion_r325677965 ## File path: common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala ## @@ -64,43 +64,34 @@ case class ActivationMessage(override val transid: TransactionId, def causedBySequence: Boolean = cause.isDefined } -object ActivationMessage extends DefaultJsonProtocol { - - def parse(msg: String) = Try(serdes.read(msg.parseJson)) - - private implicit val fqnSerdes = FullyQualifiedEntityName.serdes - implicit val serdes = jsonFormat11(ActivationMessage.apply) -} - /** * Message that is sent from the invoker to the controller after action is completed or after slot is free again for * new actions. */ abstract class AcknowledegmentMessage(private val tid: TransactionId) extends Message { override val transid: TransactionId = tid - override def serialize: String = { -AcknowledegmentMessage.serdes.write(this).compactPrint - } + override def serialize: String = AcknowledegmentMessage.serdes.write(this).compactPrint + def toJson: JsValue } /** - * This message is sent from the invoker to the controller, after the slot of an invoker that has been used by the - * current action, is free again (after log collection) + * This message is sent from an invoker to the controller, after the resource slot in the invoke, used by the + * corresponding activation, is free again (i.e., after log collection). In some cases, the activation result is + * ready and the slot is freed at the same time. In such cases, the completion message carries the result as well. + * This is reflected by the of a Right() `response` and the param `result` is set to true. + * In some cases the `result` is true but the response is Left() if the message was too large for the message bus. */ case class CompletionMessage(override val transid: TransactionId, - activationId: ActivationId, - isSystemError: Boolean, + response: Either[ActivationId, WhiskActivation], + result: Boolean, // true iff the message is a combined active ack and slot released Review comment: @rabbah As you change the current design I would also like to highlight a potential change I need for my activation persister service work (#4632) Per my current understand the `sendActiveAck` flow is like - `isSlotFree` false - active ack/ResultMessage - blocking - `WhiskActivation` - non-blocking - NONE - `isSlotFree` true - result/CompletionMessage - blocking - `CompletionMessage` - non-blocking - `CompletionMessage` Later I would like to have a configurable support for sending `WhiskActivation` for non blocking call as well to a custom topic. Just wanted to make you aware on that 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] chetanmeh opened a new pull request #4632: Activation Persister Service
chetanmeh opened a new pull request #4632: Activation Persister Service URL: https://github.com/apache/openwhisk/pull/4632 This PR introduces a new micro service - `Activation Persister Service`. This is based on discussion done [here][1] and [here][2]. ## Description This service fullfills following objective 1. Move out the load of persisting Activation results from Invoker to a new service 2. Enables a more controlled write to backend db in case rate of production of activation is more than the rate at which db can persist them within given capacity With this service the activation results would now instead written to a Kafka topic and then picked by this service and then saved in db. The approach taken here varies with following parameters 1. Is the activation for a blocking or non blocking call 2. Is the activation result being persisted from Controller (for sequence/composition) or from Invoker 3. Does the setup stores logs as part of activation or store them seprately The implementation consist of following high level components 1. Activation Persister Service - A new service impl in `core/persister` module. It is based on [Alpakka Kafka][3] and reads the activation record from Kafka and saves them to db 2. `KafkaActivationStore` - An extension of `ArtifactActivationStore` which also routes to Kafka via `MessageProducer` connector 3. InvokerReactive - The `send` logic would be modfied (details below) 4. New Kafka topic `completed-non-blocking` Some key aspects which impact the implementation are 1. Blocking/Non Blocking - Depending on invocation type the nature of activation result sent via Kafka `completed` topic varies a. Blocking - The `ResultMessage` contains the activation result without logs b. Non Blocking - Only `CompletionMessage` is sent ### Activation Persister Service This service implements a streaming flow based on [Alpakka Kafka][3]. It runs in 2 modes ActiationResult without Logs* If the setup does not store the activation with logs then persister service would listen for `ResultMessage` on all `completed.*` topics. This would also pickup records from `completed-non-blocking` topic. Further `KafkaActivationStore` behaviour would change depending on where its being used 1. Within `Controller` - Store all activations to `completed-non-blocking` (??) 2. Within `Invoker` - Noop. It would just drop all `store` calls InvokerReactive send flow would send all WhiskActivation result for blocking calls to existing `completed` topic. In addition non blocking results would also get routed to Kafka to `completed-non-blocking` ActiationResult with Logs TBD [1]: https://lists.apache.org/thread.html/7c14d2075e4f8258ec6e909f6b443f4b899b9982103a29b520d963ac@%3Cdev.openwhisk.apache.org%3E [2]:https://lists.apache.org/thread.html/0f51397cfe6a910e67b8400f8dbbf07ea468b8c3b31c771510a8@%3Cdev.openwhisk.apache.org%3E [3]: https://doc.akka.io/docs/alpakka-kafka/current/home.html ## Related issue and scope - [ ] I opened an issue to propose and discuss this change (#) ## My changes affect the following components - [ ] API - [ ] Controller - [ ] Message Bus (e.g., Kafka) - [ ] Loadbalancer - [ ] Invoker - [ ] Intrinsic actions (e.g., sequences, conductors) - [ ] Data stores (e.g., CouchDB) - [ ] Tests - [ ] Deployment - [ ] CLI - [ ] General tooling - [ ] Documentation ## Types of changes - [ ] Bug fix (generally a non-breaking change which closes an issue). - [ ] Enhancement or new feature (adds new functionality). - [ ] Breaking change (a bug fix or enhancement which changes existing behavior). ## Checklist: - [ ] I signed an [Apache CLA](https://github.com/apache/openwhisk/blob/master/CONTRIBUTING.md). - [ ] I reviewed the [style guides](https://github.com/apache/openwhisk/wiki/Contributing:-Git-guidelines#code-readiness) and followed the recommendations (Travis CI will check :). - [ ] I added tests to cover my changes. - [ ] My changes require further changes to the documentation. - [ ] I updated the documentation where necessary. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] dgrove-oss commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families
dgrove-oss commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families URL: https://github.com/apache/openwhisk/pull/4627#discussion_r325690887 ## File path: docs/actions-update.md ## @@ -0,0 +1,75 @@ + +## Updating Action Language Runtimes + +OpenWhisk supports [several languages and runtimes](actions.md#languages-and-runtimes) that can be made +available for usage in an OpenWhisk deployment. This is done via the [runtimes manifest](actions-new.md#the-runtimes-manifest). + +Over time, you may have the need for change: + +* Update runtimes to address security issues - for example, the latest code level of Node.js 10. +* Remove runtime versions that are no more supported - for example, Node.js 6. +* Add more languages due to user demand. +* Remove languages that are no more needed. + +While adding and updating languages and runtimes is pretty straightforward, removing or renaming languages and runtimes +requires some caution to prevent problems with existing actions. + +### Updating runtimes + +Follow these steps to update a particular runtime kind: + +1. Update the runtimes' container image. +2. Update the corresponding `image` property in the [runtimes manifest](actions-new.md#the-runtimes-manifest) to use the new container image. +3. Restart / re-deploy controllers and invokers such that they pick up the changed runtimes manifest. + +Already existing actions of the changed runtime kind will immediately use the new container image when the invoker creates a new action container. + +Apparently, this approach should only be used if existing actions do not break with the new container image. If a new container image is supposed to break existing actions, introduce a new runtime kind. + +### Removing runtimes + +Follow these steps to remove a particular runtime kind under the assumption that actions with the runtime kind exist in the system. Apparently, it makes sense to wait some time between accomplishing these steps to give action owners time to react. + +1. Deprecate the runtime kind by setting `"deprecated": true` in the [runtimes manifest](actions-new.md#the-runtimes-manifest). This setting prevents that new actions can be created with the deprecated action kind. In addition, existing actions cannot be changed to the deprecated action kind any more. +2. Ask owners of existing actions with runtime kind to be removed to update their actions to a different action kind. +3. Create an automated process that identifies all actions with the runtime kind to be removed in the system's action artifact store. Either automatically remove these actions or change to a different runtime kind. +4. Once the system's action artifact store does no more contain actions with the runtime kind to be removed, remove the runtime kind from the [runtimes manifest](actions-new.md#the-runtimes-manifest). +5. TODO - is this correct?: Remove the runtime kind from the list of known kinds in the `ActionExec` object of the [controller API's Swagger definition](../core/controller/src/main/resources/apiv1swagger.json). Review comment: I agree with @sven-lange-last's line of thought. runtimes.json is the source of truth, so we should be using it via the controller's REST API, not an out-of-synch swagger. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] sven-lange-last commented on a change in pull request #4628: Embedded Kafka support in OpenWhisk Standalone mode
sven-lange-last commented on a change in pull request #4628: Embedded Kafka support in OpenWhisk Standalone mode URL: https://github.com/apache/openwhisk/pull/4628#discussion_r325710115 ## File path: core/standalone/src/main/resources/logback-standalone.xml ## @@ -29,6 +29,8 @@ Review comment: I guess the existing definition for `org.apache.kafka` is for Kafka clients (consumer, producer)? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] chetanmeh commented on a change in pull request #4628: Embedded Kafka support in OpenWhisk Standalone mode
chetanmeh commented on a change in pull request #4628: Embedded Kafka support in OpenWhisk Standalone mode URL: https://github.com/apache/openwhisk/pull/4628#discussion_r325713056 ## File path: core/standalone/src/main/resources/logback-standalone.xml ## @@ -29,6 +29,8 @@ Review comment: Yes 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] tysonnorris merged pull request #4604: Allow decision about action result inclusion in logs on a per call basis
tysonnorris merged pull request #4604: Allow decision about action result inclusion in logs on a per call basis URL: https://github.com/apache/openwhisk/pull/4604 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] rabbah commented on a change in pull request #4624: Combines active ack and slot release when both are available.
rabbah commented on a change in pull request #4624: Combines active ack and slot release when both are available. URL: https://github.com/apache/openwhisk/pull/4624#discussion_r325736845 ## File path: common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala ## @@ -64,43 +64,34 @@ case class ActivationMessage(override val transid: TransactionId, def causedBySequence: Boolean = cause.isDefined } -object ActivationMessage extends DefaultJsonProtocol { - - def parse(msg: String) = Try(serdes.read(msg.parseJson)) - - private implicit val fqnSerdes = FullyQualifiedEntityName.serdes - implicit val serdes = jsonFormat11(ActivationMessage.apply) -} - /** * Message that is sent from the invoker to the controller after action is completed or after slot is free again for * new actions. */ abstract class AcknowledegmentMessage(private val tid: TransactionId) extends Message { override val transid: TransactionId = tid - override def serialize: String = { -AcknowledegmentMessage.serdes.write(this).compactPrint - } + override def serialize: String = AcknowledegmentMessage.serdes.write(this).compactPrint + def toJson: JsValue } /** - * This message is sent from the invoker to the controller, after the slot of an invoker that has been used by the - * current action, is free again (after log collection) + * This message is sent from an invoker to the controller, after the resource slot in the invoke, used by the + * corresponding activation, is free again (i.e., after log collection). In some cases, the activation result is + * ready and the slot is freed at the same time. In such cases, the completion message carries the result as well. + * This is reflected by the of a Right() `response` and the param `result` is set to true. + * In some cases the `result` is true but the response is Left() if the message was too large for the message bus. */ case class CompletionMessage(override val transid: TransactionId, - activationId: ActivationId, - isSystemError: Boolean, + response: Either[ActivationId, WhiskActivation], + result: Boolean, // true iff the message is a combined active ack and slot released Review comment: @chetanmeh I pushed a commit to implement the new type discussed earlier. I did not see your comment earlier. Do the changes help? I think it makes clearer where split-phase acks are being used and where they aren't. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] dgrove-oss commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families
dgrove-oss commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families URL: https://github.com/apache/openwhisk/pull/4627#discussion_r325740561 ## File path: docs/actions-update.md ## @@ -0,0 +1,75 @@ + +## Updating Action Language Runtimes + +OpenWhisk supports [several languages and runtimes](actions.md#languages-and-runtimes) that can be made +available for usage in an OpenWhisk deployment. This is done via the [runtimes manifest](actions-new.md#the-runtimes-manifest). + +Over time, you may have the need for change: + +* Update runtimes to address security issues - for example, the latest code level of Node.js 10. +* Remove runtime versions that are no more supported - for example, Node.js 6. +* Add more languages due to user demand. +* Remove languages that are no more needed. + +While adding and updating languages and runtimes is pretty straightforward, removing or renaming languages and runtimes +requires some caution to prevent problems with existing actions. + +### Updating runtimes + +Follow these steps to update a particular runtime kind: + +1. Update the runtimes' container image. +2. Update the corresponding `image` property in the [runtimes manifest](actions-new.md#the-runtimes-manifest) to use the new container image. +3. Restart / re-deploy controllers and invokers such that they pick up the changed runtimes manifest. + +Already existing actions of the changed runtime kind will immediately use the new container image when the invoker creates a new action container. + +Apparently, this approach should only be used if existing actions do not break with the new container image. If a new container image is supposed to break existing actions, introduce a new runtime kind. + +### Removing runtimes + +Follow these steps to remove a particular runtime kind under the assumption that actions with the runtime kind exist in the system. Apparently, it makes sense to wait some time between accomplishing these steps to give action owners time to react. + +1. Deprecate the runtime kind by setting `"deprecated": true` in the [runtimes manifest](actions-new.md#the-runtimes-manifest). This setting prevents that new actions can be created with the deprecated action kind. In addition, existing actions cannot be changed to the deprecated action kind any more. +2. Ask owners of existing actions with runtime kind to be removed to update their actions to a different action kind. +3. Create an automated process that identifies all actions with the runtime kind to be removed in the system's action artifact store. Either automatically remove these actions or change to a different runtime kind. +4. Once the system's action artifact store does no more contain actions with the runtime kind to be removed, remove the runtime kind from the [runtimes manifest](actions-new.md#the-runtimes-manifest). +5. TODO - is this correct?: Remove the runtime kind from the list of known kinds in the `ActionExec` object of the [controller API's Swagger definition](../core/controller/src/main/resources/apiv1swagger.json). Review comment: As discussed on tech exchange, updating the swagger and the implications of that is best done in a separate follow up PR. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] duynguyen commented on a change in pull request #4503: Add optional config for appending custom registry to user provided images
duynguyen commented on a change in pull request #4503: Add optional config for appending custom registry to user provided images URL: https://github.com/apache/openwhisk/pull/4503#discussion_r325750386 ## File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/docker/DockerContainerFactory.scala ## @@ -60,10 +62,14 @@ class DockerContainerFactory(instance: InvokerInstanceId, userProvidedImage: Boolean, memory: ByteSize, cpuShares: Int)(implicit config: WhiskConfig, logging: Logging): Future[Container] = { +val registryConfig = + ContainerFactory.resolveRegisterConfig(userProvidedImage, runtimesRegistryConfig, userImagesRegistryConfig) +val image = + if (userProvidedImage && registryConfig.url.isEmpty) Left(actionImage) + else Right(actionImage.localImageName(registryConfig.url)) Review comment: ok, I think what you said makes sense. thanks @sven-lange-last. Please see the latest PR version. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] codecov-io edited a comment on issue #4624: Combines active ack and slot release when both are available.
codecov-io edited a comment on issue #4624: Combines active ack and slot release when both are available. URL: https://github.com/apache/openwhisk/pull/4624#issuecomment-531316564 # [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=h1) Report > Merging [#4624](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=desc) into [master](https://codecov.io/gh/apache/openwhisk/commit/e9dd2073cd2343545d006cd71ca01dc24114c216?src=pr&el=desc) will **increase** coverage by `35.84%`. > The diff coverage is `83.95%`. [![Impacted file tree graph](https://codecov.io/gh/apache/openwhisk/pull/4624/graphs/tree.svg?width=650&token=l0YmsiSAso&height=150&src=pr)](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master#4624 +/- ## === + Coverage 42.92% 78.77% +35.84% === Files 183 183 Lines8305 8338 +33 Branches 573 572-1 === + Hits 3565 6568 +3003 + Misses 4740 1770 -2970 ``` | [Impacted Files](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [...enwhisk/core/loadBalancer/CommonLoadBalancer.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvbG9hZEJhbGFuY2VyL0NvbW1vbkxvYWRCYWxhbmNlci5zY2FsYQ==) | `81.08% <100%> (-0.17%)` | :arrow_down: | | [.../openwhisk/core/containerpool/ContainerProxy.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udGFpbmVycG9vbC9Db250YWluZXJQcm94eS5zY2FsYQ==) | `92.88% <100%> (+21.4%)` | :arrow_up: | | [...pache/openwhisk/core/invoker/InvokerReactive.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvaW52b2tlci9JbnZva2VyUmVhY3RpdmUuc2NhbGE=) | `76.66% <63.15%> (+13.18%)` | :arrow_up: | | [.../org/apache/openwhisk/core/connector/Message.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29ubmVjdG9yL01lc3NhZ2Uuc2NhbGE=) | `72.72% <86.95%> (+40.29%)` | :arrow_up: | | [...openwhisk/common/tracing/OpenTracingProvider.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvbW1vbi90cmFjaW5nL09wZW5UcmFjaW5nUHJvdmlkZXIuc2NhbGE=) | `21.15% <0%> (+1.92%)` | :arrow_up: | | [...re/database/MultipleReadersSingleWriterCache.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvTXVsdGlwbGVSZWFkZXJzU2luZ2xlV3JpdGVyQ2FjaGUuc2NhbGE=) | `98% <0%> (+2%)` | :arrow_up: | | [...apache/openwhisk/core/entitlement/Collection.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXRsZW1lbnQvQ29sbGVjdGlvbi5zY2FsYQ==) | `87.5% <0%> (+2.5%)` | :arrow_up: | | [.../org/apache/openwhisk/http/PoolingRestClient.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2h0dHAvUG9vbGluZ1Jlc3RDbGllbnQuc2NhbGE=) | `91.17% <0%> (+2.94%)` | :arrow_up: | | [...abase/cosmosdb/CosmosDBArtifactStoreProvider.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlUHJvdmlkZXIuc2NhbGE=) | `4% <0%> (+4%)` | :arrow_up: | | [...whisk/connector/kafka/KafkaConsumerConnector.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2Nvbm5lY3Rvci9rYWZrYS9LYWZrYUNvbnN1bWVyQ29ubmVjdG9yLnNjYWxh) | `57.74% <0%> (+4.22%)` | :arrow_up: | | ... and [126 more](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=footer). Last update [e9dd207...8611a8f](https://codecov.io/gh/apache/openwhisk
[GitHub] [openwhisk] codecov-io commented on issue #4503: Add optional config for appending custom registry to user provided images
codecov-io commented on issue #4503: Add optional config for appending custom registry to user provided images URL: https://github.com/apache/openwhisk/pull/4503#issuecomment-532761977 # [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4503?src=pr&el=h1) Report > Merging [#4503](https://codecov.io/gh/apache/openwhisk/pull/4503?src=pr&el=desc) into [master](https://codecov.io/gh/apache/openwhisk/commit/8d2aeabf56667a9dec49690da1cd63f64eb217ee?src=pr&el=desc) will **decrease** coverage by `5.67%`. > The diff coverage is `87.5%`. [![Impacted file tree graph](https://codecov.io/gh/apache/openwhisk/pull/4503/graphs/tree.svg?width=650&token=l0YmsiSAso&height=150&src=pr)](https://codecov.io/gh/apache/openwhisk/pull/4503?src=pr&el=tree) ```diff @@Coverage Diff @@ ## master#4503 +/- ## == - Coverage 84.45% 78.78% -5.68% == Files 183 183 Lines8305 8309 +4 Branches 581 569 -12 == - Hits 7014 6546 -468 - Misses 1291 1763 +472 ``` | [Impacted Files](https://codecov.io/gh/apache/openwhisk/pull/4503?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [...penwhisk/core/containerpool/ContainerFactory.scala](https://codecov.io/gh/apache/openwhisk/pull/4503/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udGFpbmVycG9vbC9Db250YWluZXJGYWN0b3J5LnNjYWxh) | `86.66% <ø> (+2.05%)` | :arrow_up: | | [...pool/docker/StandaloneDockerContainerFactory.scala](https://codecov.io/gh/apache/openwhisk/pull/4503/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udGFpbmVycG9vbC9kb2NrZXIvU3RhbmRhbG9uZURvY2tlckNvbnRhaW5lckZhY3Rvcnkuc2NhbGE=) | `0% <0%> (ø)` | :arrow_up: | | [...erpool/kubernetes/KubernetesContainerFactory.scala](https://codecov.io/gh/apache/openwhisk/pull/4503/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udGFpbmVycG9vbC9rdWJlcm5ldGVzL0t1YmVybmV0ZXNDb250YWluZXJGYWN0b3J5LnNjYWxh) | `0% <0%> (ø)` | :arrow_up: | | [...che/openwhisk/core/yarn/YARNContainerFactory.scala](https://codecov.io/gh/apache/openwhisk/pull/4503/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUveWFybi9ZQVJOQ29udGFpbmVyRmFjdG9yeS5zY2FsYQ==) | `87.5% <100%> (ø)` | :arrow_up: | | [.../containerpool/docker/DockerContainerFactory.scala](https://codecov.io/gh/apache/openwhisk/pull/4503/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udGFpbmVycG9vbC9kb2NrZXIvRG9ja2VyQ29udGFpbmVyRmFjdG9yeS5zY2FsYQ==) | `69.69% <100%> (+1.95%)` | :arrow_up: | | [...rg/apache/openwhisk/core/entity/ExecManifest.scala](https://codecov.io/gh/apache/openwhisk/pull/4503/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXR5L0V4ZWNNYW5pZmVzdC5zY2FsYQ==) | `97.84% <100%> (-0.03%)` | :arrow_down: | | [...sk/core/containerpool/docker/DockerContainer.scala](https://codecov.io/gh/apache/openwhisk/pull/4503/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udGFpbmVycG9vbC9kb2NrZXIvRG9ja2VyQ29udGFpbmVyLnNjYWxh) | `96.47% <100%> (+0.04%)` | :arrow_up: | | [...e/openwhisk/core/mesos/MesosContainerFactory.scala](https://codecov.io/gh/apache/openwhisk/pull/4503/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvbWVzb3MvTWVzb3NDb250YWluZXJGYWN0b3J5LnNjYWxh) | `60.71% <100%> (+1.78%)` | :arrow_up: | | [.../scala/org/apache/openwhisk/core/entity/Exec.scala](https://codecov.io/gh/apache/openwhisk/pull/4503/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXR5L0V4ZWMuc2NhbGE=) | `90.47% <100%> (ø)` | :arrow_up: | | [.../scala/org/apache/openwhisk/core/WhiskConfig.scala](https://codecov.io/gh/apache/openwhisk/pull/4503/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvV2hpc2tDb25maWcuc2NhbGE=) | `94.73% <100%> (+0.03%)` | :arrow_up: | | ... and [24 more](https://codecov.io/gh/apache/openwhisk/pull/4503/diff?src=pr&el=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/openwhisk/pull/4503?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4503?src=pr&el=footer). Last update [8d2aeab...29ab3b4](https://codecov.io/gh/apache/openwhisk/pull/4503?s
[GitHub] [openwhisk-utilities] mrutkows merged pull request #72: Improve pre-commit sample for openwhisk
mrutkows merged pull request #72: Improve pre-commit sample for openwhisk URL: https://github.com/apache/openwhisk-utilities/pull/72 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk-release] dgrove-oss opened a new pull request #302: exclude empty files in the find command; not by mime type
dgrove-oss opened a new pull request #302: exclude empty files in the find command; not by mime type URL: https://github.com/apache/openwhisk-release/pull/302 Apparently the mime type reported by file of a zero byte file is platform dependent; so it is better to use a smarter find command than to filter out later... 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk-release] dgrove-oss commented on issue #302: exclude empty files in the find command; not by mime type
dgrove-oss commented on issue #302: exclude empty files in the find command; not by mime type URL: https://github.com/apache/openwhisk-release/pull/302#issuecomment-532789924 @style95 please see if this variant works on your platform 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk-runtime-go] lionelvillard opened a new issue #104: Cannot build the runrime
lionelvillard opened a new issue #104: Cannot build the runrime URL: https://github.com/apache/openwhisk-runtime-go/issues/104 I followed [these instructions](https://github.com/apache/openwhisk-runtime-go/blob/master/docs/BUILD.md#how-to-build-and-run-tests). I'm getting this error: ``` FAILURE: Build failed with an exception. * What went wrong: Could not resolve all files for configuration ':tests:testCompileClasspath'. > Could not find org.apache.openwhisk:openwhisk-tests:1.0.0-SNAPSHOT. Searched in the following locations: https://repo1.maven.org/maven2/org/apache/openwhisk/openwhisk-tests/1.0.0-SNAPSHOT/maven-metadata.xml https://repo1.maven.org/maven2/org/apache/openwhisk/openwhisk-tests/1.0.0-SNAPSHOT/openwhisk-tests-1.0.0-SNAPSHOT.pom https://repo1.maven.org/maven2/org/apache/openwhisk/openwhisk-tests/1.0.0-SNAPSHOT/openwhisk-tests-1.0.0-SNAPSHOT-tests.jar Required by: project :tests ``` 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk-runtime-go] lionelvillard commented on issue #104: Cannot build the runtime
lionelvillard commented on issue #104: Cannot build the runtime URL: https://github.com/apache/openwhisk-runtime-go/issues/104#issuecomment-532830323 note that `./gradlew distDocker` works as expected. Maybe `./gradlew build` is not needed anymore and the readme should be fixed? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk-release] style95 merged pull request #302: exclude empty files in the find command; not by mime type
style95 merged pull request #302: exclude empty files in the find command; not by mime type URL: https://github.com/apache/openwhisk-release/pull/302 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] chetanmeh commented on a change in pull request #4624: Combines active ack and slot release when both are available.
chetanmeh commented on a change in pull request #4624: Combines active ack and slot release when both are available. URL: https://github.com/apache/openwhisk/pull/4624#discussion_r325989534 ## File path: common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala ## @@ -64,100 +64,165 @@ case class ActivationMessage(override val transid: TransactionId, def causedBySequence: Boolean = cause.isDefined } -object ActivationMessage extends DefaultJsonProtocol { - - def parse(msg: String) = Try(serdes.read(msg.parseJson)) - - private implicit val fqnSerdes = FullyQualifiedEntityName.serdes - implicit val serdes = jsonFormat11(ActivationMessage.apply) -} - /** * Message that is sent from the invoker to the controller after action is completed or after slot is free again for * new actions. */ abstract class AcknowledegmentMessage(private val tid: TransactionId) extends Message { override val transid: TransactionId = tid - override def serialize: String = { -AcknowledegmentMessage.serdes.write(this).compactPrint - } + override def serialize: String = AcknowledegmentMessage.serdes.write(this).compactPrint + + /** Pithy descriptor for logging. */ + def name: String + + /** Does message indicate slot is free? */ + def isSlotFree: Option[InvokerInstanceId] + + /** Does message contain a result? */ + def result: Option[Either[ActivationId, WhiskActivation]] + + /** + * Is the acknowledgement for an activation that failed internally? + * For some message, this is not relevant and the result is None. + */ + def isSystemError: Option[Boolean] + + def activationId: ActivationId + + /** Serializes the message to JSON. */ + def toJson: JsValue + + /** + * Converts the message to a more compact form if it cannot cross the message bus as is or some of its details are not necessary. + */ + def shrink: AcknowledegmentMessage } /** - * This message is sent from the invoker to the controller, after the slot of an invoker that has been used by the - * current action, is free again (after log collection) + * This message is sent from an invoker to the controller in situtations when the resource slot and the action + * result are available at the same time, and so the split-phase notification is not necessary. Instead the message + * combines the `CompletionMessage` and `ResultMessage`. The `response` may be an `ActivationId` to allow for failures + * to send the activation result because of event-bus size limitations. */ -case class CompletionMessage(override val transid: TransactionId, - activationId: ActivationId, - isSystemError: Boolean, - invoker: InvokerInstanceId) +case class CombinedCompletionAndResultMessage(override val transid: TransactionId, + response: Either[ActivationId, WhiskActivation], + override val isSystemError: Option[Boolean], + invoker: InvokerInstanceId) extends AcknowledegmentMessage(transid) { - - override def toString = { -activationId.asString - } + override def name = "combined" + override def result = Some(response) + override def isSlotFree = Some(invoker) + override def activationId = response.fold(identity, _.activationId) + override def toJson = CombinedCompletionAndResultMessage.serdes.write(this) + override def shrink = copy(response = response.flatMap(a => Left(a.activationId))) + override def toString = response.fold(identity, _.activationId).asString Review comment: Nit: Reuse `activationId.asString` 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] chetanmeh commented on a change in pull request #4624: Combines active ack and slot release when both are available.
chetanmeh commented on a change in pull request #4624: Combines active ack and slot release when both are available. URL: https://github.com/apache/openwhisk/pull/4624#discussion_r325993909 ## File path: common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala ## @@ -64,100 +64,165 @@ case class ActivationMessage(override val transid: TransactionId, def causedBySequence: Boolean = cause.isDefined } -object ActivationMessage extends DefaultJsonProtocol { - - def parse(msg: String) = Try(serdes.read(msg.parseJson)) - - private implicit val fqnSerdes = FullyQualifiedEntityName.serdes - implicit val serdes = jsonFormat11(ActivationMessage.apply) -} - /** * Message that is sent from the invoker to the controller after action is completed or after slot is free again for * new actions. */ abstract class AcknowledegmentMessage(private val tid: TransactionId) extends Message { override val transid: TransactionId = tid - override def serialize: String = { -AcknowledegmentMessage.serdes.write(this).compactPrint - } + override def serialize: String = AcknowledegmentMessage.serdes.write(this).compactPrint + + /** Pithy descriptor for logging. */ + def name: String + + /** Does message indicate slot is free? */ + def isSlotFree: Option[InvokerInstanceId] + + /** Does message contain a result? */ + def result: Option[Either[ActivationId, WhiskActivation]] + + /** + * Is the acknowledgement for an activation that failed internally? + * For some message, this is not relevant and the result is None. + */ + def isSystemError: Option[Boolean] + + def activationId: ActivationId + + /** Serializes the message to JSON. */ + def toJson: JsValue + + /** + * Converts the message to a more compact form if it cannot cross the message bus as is or some of its details are not necessary. + */ + def shrink: AcknowledegmentMessage } /** - * This message is sent from the invoker to the controller, after the slot of an invoker that has been used by the - * current action, is free again (after log collection) + * This message is sent from an invoker to the controller in situtations when the resource slot and the action + * result are available at the same time, and so the split-phase notification is not necessary. Instead the message + * combines the `CompletionMessage` and `ResultMessage`. The `response` may be an `ActivationId` to allow for failures + * to send the activation result because of event-bus size limitations. */ -case class CompletionMessage(override val transid: TransactionId, - activationId: ActivationId, - isSystemError: Boolean, - invoker: InvokerInstanceId) +case class CombinedCompletionAndResultMessage(override val transid: TransactionId, + response: Either[ActivationId, WhiskActivation], + override val isSystemError: Option[Boolean], + invoker: InvokerInstanceId) extends AcknowledegmentMessage(transid) { - - override def toString = { -activationId.asString - } + override def name = "combined" + override def result = Some(response) + override def isSlotFree = Some(invoker) + override def activationId = response.fold(identity, _.activationId) + override def toJson = CombinedCompletionAndResultMessage.serdes.write(this) + override def shrink = copy(response = response.flatMap(a => Left(a.activationId))) + override def toString = response.fold(identity, _.activationId).asString } -object CompletionMessage extends DefaultJsonProtocol { - def parse(msg: String): Try[CompletionMessage] = Try(serdes.read(msg.parseJson)) - implicit val serdes = jsonFormat4(CompletionMessage.apply) +/** + * This message is sent from an invoker to the controller, once the resource slot in the invoker (used by the + * corresponding activation) free again (i.e., after log collection). The `CompletionMessage` is part of a split + * phase notification to the load balancer where an invoker first sends a `ResultMessage` and later sends the + * `CompletionMessage`. + */ +case class CompletionMessage(override val transid: TransactionId, + override val activationId: ActivationId, + override val isSystemError: Option[Boolean], + invoker: InvokerInstanceId) +extends AcknowledegmentMessage(transid) { + override def name = "completion" + override def result = None + override def isSlotFree = Some(invoker) + override def toJson = CompletionMessage.serdes.write(this) + override def shrink = this + override def toString = activationId.asString } /** - * That message will be sent from the invoker to the controller after action completion if the user wants to have - * the result immediately (blocking activation).
[GitHub] [openwhisk] chetanmeh commented on a change in pull request #4624: Combines active ack and slot release when both are available.
chetanmeh commented on a change in pull request #4624: Combines active ack and slot release when both are available. URL: https://github.com/apache/openwhisk/pull/4624#discussion_r325988736 ## File path: common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala ## @@ -64,100 +64,165 @@ case class ActivationMessage(override val transid: TransactionId, def causedBySequence: Boolean = cause.isDefined } -object ActivationMessage extends DefaultJsonProtocol { - - def parse(msg: String) = Try(serdes.read(msg.parseJson)) - - private implicit val fqnSerdes = FullyQualifiedEntityName.serdes - implicit val serdes = jsonFormat11(ActivationMessage.apply) -} - /** * Message that is sent from the invoker to the controller after action is completed or after slot is free again for * new actions. */ abstract class AcknowledegmentMessage(private val tid: TransactionId) extends Message { override val transid: TransactionId = tid - override def serialize: String = { -AcknowledegmentMessage.serdes.write(this).compactPrint - } + override def serialize: String = AcknowledegmentMessage.serdes.write(this).compactPrint + + /** Pithy descriptor for logging. */ + def name: String Review comment: Message type makes more sense compared to message name. So may be rename to `messageType` 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] chetanmeh commented on a change in pull request #4624: Combines active ack and slot release when both are available.
chetanmeh commented on a change in pull request #4624: Combines active ack and slot release when both are available. URL: https://github.com/apache/openwhisk/pull/4624#discussion_r325996077 ## File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerProxy.scala ## @@ -628,8 +637,15 @@ class ContainerProxy( // completion message which frees a load balancer slot is sent after the active ack future // completes to ensure proper ordering. val sendResult = if (job.msg.blocking) { - activation.map( -sendActiveAck(tid, _, job.msg.blocking, job.msg.rootControllerIndex, job.msg.user.namespace.uuid, false)) + activation.map { result => +sendActiveAck( + tid, + result, + job.msg.blocking, + job.msg.rootControllerIndex, + job.msg.user.namespace.uuid, + ResultMessage(tid, result)) Review comment: Later optimization: Here we can also send `CombinedCompletionAndResultMessage` if we know that logs are not to be collected for 1. `job.action.limits.logs.asMegaBytes == 0.MB` 2. OR `LogDriverLogStore` is being used Specially in `LogDriverLogStore` based setup then `ResultMessage` would not be used then 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] chetanmeh commented on a change in pull request #4624: Combines active ack and slot release when both are available.
chetanmeh commented on a change in pull request #4624: Combines active ack and slot release when both are available. URL: https://github.com/apache/openwhisk/pull/4624#discussion_r325988193 ## File path: common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala ## @@ -64,100 +64,165 @@ case class ActivationMessage(override val transid: TransactionId, def causedBySequence: Boolean = cause.isDefined } -object ActivationMessage extends DefaultJsonProtocol { - - def parse(msg: String) = Try(serdes.read(msg.parseJson)) - - private implicit val fqnSerdes = FullyQualifiedEntityName.serdes - implicit val serdes = jsonFormat11(ActivationMessage.apply) -} - /** * Message that is sent from the invoker to the controller after action is completed or after slot is free again for * new actions. */ abstract class AcknowledegmentMessage(private val tid: TransactionId) extends Message { override val transid: TransactionId = tid - override def serialize: String = { -AcknowledegmentMessage.serdes.write(this).compactPrint - } + override def serialize: String = AcknowledegmentMessage.serdes.write(this).compactPrint + + /** Pithy descriptor for logging. */ + def name: String + + /** Does message indicate slot is free? */ + def isSlotFree: Option[InvokerInstanceId] + + /** Does message contain a result? */ + def result: Option[Either[ActivationId, WhiskActivation]] + + /** + * Is the acknowledgement for an activation that failed internally? + * For some message, this is not relevant and the result is None. + */ + def isSystemError: Option[Boolean] + + def activationId: ActivationId + + /** Serializes the message to JSON. */ + def toJson: JsValue + + /** + * Converts the message to a more compact form if it cannot cross the message bus as is or some of its details are not necessary. + */ + def shrink: AcknowledegmentMessage } /** - * This message is sent from the invoker to the controller, after the slot of an invoker that has been used by the - * current action, is free again (after log collection) + * This message is sent from an invoker to the controller in situtations when the resource slot and the action Review comment: Nit: `situtations` -> `situations` 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] chetanmeh commented on a change in pull request #4624: Combines active ack and slot release when both are available.
chetanmeh commented on a change in pull request #4624: Combines active ack and slot release when both are available. URL: https://github.com/apache/openwhisk/pull/4624#discussion_r325991407 ## File path: common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala ## @@ -64,100 +64,165 @@ case class ActivationMessage(override val transid: TransactionId, def causedBySequence: Boolean = cause.isDefined } -object ActivationMessage extends DefaultJsonProtocol { - - def parse(msg: String) = Try(serdes.read(msg.parseJson)) - - private implicit val fqnSerdes = FullyQualifiedEntityName.serdes - implicit val serdes = jsonFormat11(ActivationMessage.apply) -} - /** * Message that is sent from the invoker to the controller after action is completed or after slot is free again for * new actions. */ abstract class AcknowledegmentMessage(private val tid: TransactionId) extends Message { override val transid: TransactionId = tid - override def serialize: String = { -AcknowledegmentMessage.serdes.write(this).compactPrint - } + override def serialize: String = AcknowledegmentMessage.serdes.write(this).compactPrint + + /** Pithy descriptor for logging. */ + def name: String + + /** Does message indicate slot is free? */ + def isSlotFree: Option[InvokerInstanceId] + + /** Does message contain a result? */ + def result: Option[Either[ActivationId, WhiskActivation]] + + /** + * Is the acknowledgement for an activation that failed internally? + * For some message, this is not relevant and the result is None. + */ + def isSystemError: Option[Boolean] + + def activationId: ActivationId + + /** Serializes the message to JSON. */ + def toJson: JsValue + + /** + * Converts the message to a more compact form if it cannot cross the message bus as is or some of its details are not necessary. + */ + def shrink: AcknowledegmentMessage } /** - * This message is sent from the invoker to the controller, after the slot of an invoker that has been used by the - * current action, is free again (after log collection) + * This message is sent from an invoker to the controller in situtations when the resource slot and the action + * result are available at the same time, and so the split-phase notification is not necessary. Instead the message + * combines the `CompletionMessage` and `ResultMessage`. The `response` may be an `ActivationId` to allow for failures + * to send the activation result because of event-bus size limitations. */ -case class CompletionMessage(override val transid: TransactionId, - activationId: ActivationId, - isSystemError: Boolean, - invoker: InvokerInstanceId) +case class CombinedCompletionAndResultMessage(override val transid: TransactionId, + response: Either[ActivationId, WhiskActivation], + override val isSystemError: Option[Boolean], + invoker: InvokerInstanceId) extends AcknowledegmentMessage(transid) { - - override def toString = { -activationId.asString - } + override def name = "combined" + override def result = Some(response) + override def isSlotFree = Some(invoker) + override def activationId = response.fold(identity, _.activationId) + override def toJson = CombinedCompletionAndResultMessage.serdes.write(this) + override def shrink = copy(response = response.flatMap(a => Left(a.activationId))) + override def toString = response.fold(identity, _.activationId).asString } -object CompletionMessage extends DefaultJsonProtocol { - def parse(msg: String): Try[CompletionMessage] = Try(serdes.read(msg.parseJson)) - implicit val serdes = jsonFormat4(CompletionMessage.apply) +/** + * This message is sent from an invoker to the controller, once the resource slot in the invoker (used by the + * corresponding activation) free again (i.e., after log collection). The `CompletionMessage` is part of a split + * phase notification to the load balancer where an invoker first sends a `ResultMessage` and later sends the + * `CompletionMessage`. + */ +case class CompletionMessage(override val transid: TransactionId, + override val activationId: ActivationId, + override val isSystemError: Option[Boolean], + invoker: InvokerInstanceId) +extends AcknowledegmentMessage(transid) { + override def name = "completion" + override def result = None + override def isSlotFree = Some(invoker) + override def toJson = CompletionMessage.serdes.write(this) + override def shrink = this + override def toString = activationId.asString } /** - * That message will be sent from the invoker to the controller after action completion if the user wants to have - * the result immediately (blocking activation).
[GitHub] [openwhisk] chetanmeh commented on a change in pull request #4624: Combines active ack and slot release when both are available.
chetanmeh commented on a change in pull request #4624: Combines active ack and slot release when both are available. URL: https://github.com/apache/openwhisk/pull/4624#discussion_r325990234 ## File path: core/invoker/src/main/scala/org/apache/openwhisk/core/invoker/InvokerReactive.scala ## @@ -54,9 +54,10 @@ object InvokerReactive extends InvokerProvider { * @param Boolean is true iff the activation was a blocking request * @param ControllerInstanceId the originating controller/loadbalancer id * @param UUID is the UUID for the namespace owning the activation - * @param Boolean is true this is resource free message and false if this is a result forwarding message + * @param AcknowledegmentMessage the acknowledgement message to send */ - type ActiveAck = (TransactionId, WhiskActivation, Boolean, ControllerInstanceId, UUID, Boolean) => Future[Any] + type ActiveAck = Review comment: Minor observation (or Rant!) - I always struggle in IDE whenever I want to see impl of parameters which are specified as function signature instead of trait (like in `ActiveAck`). Here things are bit simpler as we specify a `type` alias which narrows down the search. Otherwise one need to check the whole call hierarchy to understand where is. the actual impl Having a trait enables easier checking of possible implementations to understand the code flow. This is pre existing stuff ... but may be later we refactor it and use a proper trait for such critical flows 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] chetanmeh opened a new issue #4633: Optimize WhiskEntity deserialization
chetanmeh opened a new issue #4633: Optimize WhiskEntity deserialization URL: https://github.com/apache/openwhisk/issues/4633 Currently the deserialization logic for `WhiskEntity` uses a trail based approahc where it tries multiple deserializers https://github.com/apache/openwhisk/blob/e9dd2073cd2343545d006cd71ca01dc24114c216/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskEntity.scala#L134-L152 This can be optimized for newer setups which started using new `entityType` attribute (#3164). So we can have a pre check if the object have field `entityType` defined then use that otherwise fallback to existing approach. It would avoid the cost of exception generated for cases where we are looking for entries later in the sequence 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] selfxp opened a new issue #4634: Extends user-events configuration
selfxp opened a new issue #4634: Extends user-events configuration URL: https://github.com/apache/openwhisk/issues/4634 * The Prometheus recorder can't be disabled in the current `user-events` implementation. Someone might want to use the KamonRecorder with some reporters other than Prometheus. It would be better to make the `reporter` and `recorder` configurable. https://github.com/apache/openwhisk/blob/f04275a825346da52d8616cf241f7a6ae78dccbc/core/monitoring/user-events/src/main/scala/org/apache/openwhisk/core/monitoring/metrics/OpenWhiskEvents.scala#L49 * Make kafka `client.id` configurable in order to be able to run multiple instances of the this service. https://github.com/apache/openwhisk/blob/f04275a825346da52d8616cf241f7a6ae78dccbc/core/monitoring/user-events/src/main/scala/org/apache/openwhisk/core/monitoring/metrics/EventConsumer.scala#L138 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services