[GitHub] codecov-io edited a comment on issue #3889: make request id header name comparison case insensitive
codecov-io edited a comment on issue #3889: make request id header name comparison case insensitive URL: https://github.com/apache/incubator-openwhisk/pull/3889#issuecomment-406744339 # [Codecov](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889?src=pr=h1) Report > Merging [#3889](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-openwhisk/commit/1b8ffcd3203b38eae045a915211ad6d84cc72638?src=pr=desc) will **decrease** coverage by `4.76%`. > The diff coverage is `0%`. [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889/graphs/tree.svg?width=650=l0YmsiSAso=150=pr)](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889?src=pr=tree) ```diff @@Coverage Diff @@ ## master#3889 +/- ## == - Coverage 75.72% 70.96% -4.77% == Files 145 145 Lines6901 6901 Branches 417 417 == - Hits 5226 4897 -329 - Misses 1675 2004 +329 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889?src=pr=tree) | Coverage Δ | | |---|---|---| | [...a/src/main/scala/whisk/http/BasicHttpService.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2h0dHAvQmFzaWNIdHRwU2VydmljZS5zY2FsYQ==) | `0% <0%> (ø)` | :arrow_up: | | [...core/database/cosmosdb/RxObservableImplicits.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvUnhPYnNlcnZhYmxlSW1wbGljaXRzLnNjYWxh) | `0% <0%> (-100%)` | :arrow_down: | | [...core/database/cosmosdb/CosmosDBArtifactStore.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlLnNjYWxh) | `0% <0%> (-95.08%)` | :arrow_down: | | [...sk/core/database/cosmosdb/CosmosDBViewMapper.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJWaWV3TWFwcGVyLnNjYWxh) | `0% <0%> (-92.6%)` | :arrow_down: | | [...whisk/core/database/cosmosdb/CosmosDBSupport.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJTdXBwb3J0LnNjYWxh) | `0% <0%> (-81.82%)` | :arrow_down: | | [...abase/cosmosdb/CosmosDBArtifactStoreProvider.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlUHJvdmlkZXIuc2NhbGE=) | `0% <0%> (-58.83%)` | :arrow_down: | | [...la/whisk/core/database/cosmosdb/CosmosDBUtil.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJVdGlsLnNjYWxh) | `92% <0%> (-4%)` | :arrow_down: | | [...rc/main/scala/whisk/common/ForcableSemaphore.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvbW1vbi9Gb3JjYWJsZVNlbWFwaG9yZS5zY2FsYQ==) | `88.46% <0%> (+3.84%)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889?src=pr=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/incubator-openwhisk/pull/3889?src=pr=footer). Last update [1b8ffcd...82036b0](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889?src=pr=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] codecov-io commented on issue #3889: make request id header name comparison case insensitive
codecov-io commented on issue #3889: make request id header name comparison case insensitive URL: https://github.com/apache/incubator-openwhisk/pull/3889#issuecomment-406744339 # [Codecov](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889?src=pr=h1) Report > Merging [#3889](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-openwhisk/commit/1b8ffcd3203b38eae045a915211ad6d84cc72638?src=pr=desc) will **decrease** coverage by `4.76%`. > The diff coverage is `0%`. [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889/graphs/tree.svg?width=650=150=pr=l0YmsiSAso)](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889?src=pr=tree) ```diff @@Coverage Diff @@ ## master#3889 +/- ## == - Coverage 75.72% 70.96% -4.77% == Files 145 145 Lines6901 6901 Branches 417 417 == - Hits 5226 4897 -329 - Misses 1675 2004 +329 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889?src=pr=tree) | Coverage Δ | | |---|---|---| | [...a/src/main/scala/whisk/http/BasicHttpService.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2h0dHAvQmFzaWNIdHRwU2VydmljZS5zY2FsYQ==) | `0% <0%> (ø)` | :arrow_up: | | [...core/database/cosmosdb/RxObservableImplicits.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvUnhPYnNlcnZhYmxlSW1wbGljaXRzLnNjYWxh) | `0% <0%> (-100%)` | :arrow_down: | | [...core/database/cosmosdb/CosmosDBArtifactStore.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlLnNjYWxh) | `0% <0%> (-95.08%)` | :arrow_down: | | [...sk/core/database/cosmosdb/CosmosDBViewMapper.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJWaWV3TWFwcGVyLnNjYWxh) | `0% <0%> (-92.6%)` | :arrow_down: | | [...whisk/core/database/cosmosdb/CosmosDBSupport.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJTdXBwb3J0LnNjYWxh) | `0% <0%> (-81.82%)` | :arrow_down: | | [...abase/cosmosdb/CosmosDBArtifactStoreProvider.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlUHJvdmlkZXIuc2NhbGE=) | `0% <0%> (-58.83%)` | :arrow_down: | | [...la/whisk/core/database/cosmosdb/CosmosDBUtil.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJVdGlsLnNjYWxh) | `92% <0%> (-4%)` | :arrow_down: | | [...rc/main/scala/whisk/common/ForcableSemaphore.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvbW1vbi9Gb3JjYWJsZVNlbWFwaG9yZS5zY2FsYQ==) | `88.46% <0%> (+3.84%)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889?src=pr=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/incubator-openwhisk/pull/3889?src=pr=footer). Last update [1b8ffcd...82036b0](https://codecov.io/gh/apache/incubator-openwhisk/pull/3889?src=pr=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ddragosd commented on issue #3889: make request id header name comparison case insensitive
ddragosd commented on issue #3889: make request id header name comparison case insensitive URL: https://github.com/apache/incubator-openwhisk/pull/3889#issuecomment-406741824 great catch @tysonnorris . Indeed, as per the RFC https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2 , _field names are case-insensitive_ , meaning `X-Request-ID`, or `x-request-id` should both work. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] tysonnorris opened a new pull request #3889: make request id header name comparison case insensitive
tysonnorris opened a new pull request #3889: make request id header name comparison case insensitive URL: https://github.com/apache/incubator-openwhisk/pull/3889 ## Description Request headers should be compared case insensitive. We noticed X-Request-Id was being ignored at controller, due to the configured header being X-Request-ID ## Related issue and scope - [ ] I opened an issue to propose and discuss this change (#) ## My changes affect the following components - [ ] API - [x] 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 - [x] 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: - [x] I signed an [Apache CLA](https://github.com/apache/incubator-openwhisk/blob/master/CONTRIBUTING.md). - [x] I reviewed the [style guides](https://github.com/apache/incubator-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 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] mdeuser opened a new issue #3888: api swagger validation throws exception on an api using the x-require-whisk-auth header
mdeuser opened a new issue #3888: api swagger validation throws exception on an api using the x-require-whisk-auth header URL: https://github.com/apache/incubator-openwhisk/issues/3888 Normally, the related swagger section defining the need for the api gw to inject the x-require-whisk-auth header looks like: ``` "execute": [ { "invoke": { "target-url": "https://openwhisk.ng.bluemix.net/api/v1/web/NAMESPACE/PACKAGE/ACTION.json;, "verb": "keep" } }, { "set-variable": { "actions": [ { "set": "message.headers.X-Require-Whisk-Auth", "value": "" } ] } } ], ``` if the `set-variable` object comes before the `invoke` object in this `execute` array, an exception is thrown by the swagger validation logic in the apimgmt/createApi web action. ``` error: Unable to create API: API creation failure: Cannot read property 'target-url' of undefined ``` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] houshengbo closed pull request #224: Update the download links of the 0.9.0 artifacts with mirror links
houshengbo closed pull request #224: Update the download links of the 0.9.0 artifacts with mirror links URL: https://github.com/apache/incubator-openwhisk-release/pull/224 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/releases/0.9.0-incubating/INSTALL.md b/releases/0.9.0-incubating/INSTALL.md index 39d2be0..3282abd 100644 --- a/releases/0.9.0-incubating/INSTALL.md +++ b/releases/0.9.0-incubating/INSTALL.md @@ -19,11 +19,11 @@ # Download OpenWhisk -The OpenWhisk source code may be downloaded from https://dist.apache.org/repos/dist/release/incubator/openwhisk/apache-openwhisk-0.9.0-incubating. The current release is version 0.9.0, and the artifact for this OpenWhisk source code release is called `"openwhisk-0.9.0-incubating-sources.tar.gz"`. +The OpenWhisk source code may be downloaded from [this link](http://www.apache.org/dyn/closer.lua?filename=incubator/openwhisk/apache-openwhisk-0.9.0-incubating/openwhisk-0.9.0-incubating-sources.tar.gz=download). The current release is version 0.9.0, and the artifact for this OpenWhisk source code release is called `"openwhisk-0.9.0-incubating-sources.tar.gz"`. ``` # download from your terminal with wget -wget https://dist.apache.org/repos/dist/release/incubator/openwhisk/apache-openwhisk-0.9.0-incubating/openwhisk-0.9.0-incubating-sources.tar.gz +wget http://apache.mirrors.ionfish.org/incubator/openwhisk/apache-openwhisk-0.9.0-incubating/openwhisk-0.9.0-incubating-sources.tar.gz ``` # Verify the SHA-512 checksums, and signature @@ -42,7 +42,7 @@ You need to install `gpg` on your local machine. Before using `gpg` to verify the OpenWhisk release integrity, you should [verify `gpg`'s own integrity](https://gnupg.org/download/integrity_check.html). -The public key used to verify the OpenWhisk checksums can be found [here](https://dist.apache.org/repos/dist/release/incubator/openwhisk/KEYS). Download the key and import it on your local machine. +The public key used to verify the OpenWhisk checksums can be found [here](https://www-us.apache.org/dist/incubator/openwhisk/KEYS). Download the key and import it on your local machine. ``` gpg --import @@ -56,9 +56,9 @@ To generate the SHA512 checksum: gpg --print-md SHA512 ``` -The parameter is the file of the artifact `"openwhisk-0.9.0-incubating-sources.tar.gz"`. Compare the content with the [SHA512 file](https://dist.apache.org/repos/dist/release/incubator/openwhisk/apache-openwhisk-0.9.0-incubating/openwhisk-0.9.0-incubating-sources.tar.gz.sha512). +The parameter is the file of the artifact `"openwhisk-0.9.0-incubating-sources.tar.gz"`. Compare the content with the [SHA512 file](https://www-us.apache.org/dist/incubator/openwhisk/apache-openwhisk-0.9.0-incubating/openwhisk-0.9.0-incubating-sources.tar.gz.sha512). -Download the [signature](https://dist.apache.org/repos/dist/release/incubator/openwhisk/apache-openwhisk-0.9.0-incubating/openwhisk-0.9.0-incubating-sources.tar.gz.asc), and verify it with the command: +Download the [signature](https://www-us.apache.org/dist/incubator/openwhisk/apache-openwhisk-0.9.0-incubating/openwhisk-0.9.0-incubating-sources.tar.gz.asc), and verify it with the command: ``` gpg --verify openwhisk-0.9.0-incubating-sources.tar.gz.asc openwhisk-0.9.0-incubating-sources.tar.gz This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] tysonnorris commented on issue #3812: ContainerClient + akka http alternative to HttpUtils
tysonnorris commented on issue #3812: ContainerClient + akka http alternative to HttpUtils URL: https://github.com/apache/incubator-openwhisk/pull/3812#issuecomment-406720039 @chetanmeh @markusthoemmes I think I have addressed all comments This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] codecov-io edited a comment on issue #3812: ContainerClient + akka http alternative to HttpUtils
codecov-io edited a comment on issue #3812: ContainerClient + akka http alternative to HttpUtils URL: https://github.com/apache/incubator-openwhisk/pull/3812#issuecomment-401212372 # [Codecov](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812?src=pr=h1) Report > Merging [#3812](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-openwhisk/commit/e96c1bbd5a0f54c923f0153116f7c1f165275e94?src=pr=desc) will **decrease** coverage by `4.67%`. > The diff coverage is `54.11%`. [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/graphs/tree.svg?width=650=150=l0YmsiSAso=pr)](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812?src=pr=tree) ```diff @@Coverage Diff @@ ## master#3812 +/- ## == - Coverage 75.78% 71.11% -4.68% == Files 145 146 +1 Lines6897 6954 +57 Branches 418 426 +8 == - Hits 5227 4945 -282 - Misses 1670 2009 +339 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812?src=pr=tree) | Coverage Δ | | |---|---|---| | [...la/whisk/core/containerpool/ContainerFactory.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvY29udGFpbmVycG9vbC9Db250YWluZXJGYWN0b3J5LnNjYWxh) | `100% <ø> (ø)` | :arrow_up: | | [...containerpool/kubernetes/KubernetesContainer.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvY29udGFpbmVycG9vbC9rdWJlcm5ldGVzL0t1YmVybmV0ZXNDb250YWluZXIuc2NhbGE=) | `91.66% <ø> (ø)` | :arrow_up: | | [...la/src/main/scala/whisk/core/mesos/MesosTask.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvbWVzb3MvTWVzb3NUYXNrLnNjYWxh) | `86.11% <ø> (ø)` | :arrow_up: | | [...sk/core/containerpool/docker/DockerContainer.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvY29udGFpbmVycG9vbC9kb2NrZXIvRG9ja2VyQ29udGFpbmVyLnNjYWxh) | `75% <0%> (-2.03%)` | :arrow_down: | | [...on/scala/src/main/scala/whisk/common/Logging.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvbW1vbi9Mb2dnaW5nLnNjYWxh) | `87.09% <100%> (+0.28%)` | :arrow_up: | | [.../src/main/scala/whisk/http/PoolingRestClient.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2h0dHAvUG9vbGluZ1Jlc3RDbGllbnQuc2NhbGE=) | `90% <100%> (+0.71%)` | :arrow_up: | | [...n/scala/whisk/core/database/CouchDbRestStore.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvQ291Y2hEYlJlc3RTdG9yZS5zY2FsYQ==) | `73.23% <100%> (ø)` | :arrow_up: | | [...ain/scala/whisk/core/containerpool/Container.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvY29udGFpbmVycG9vbC9Db250YWluZXIuc2NhbGE=) | `74.24% <20%> (-1.95%)` | :arrow_down: | | [.../containerpool/ApacheBlockingContainerClient.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvY29udGFpbmVycG9vbC9BcGFjaGVCbG9ja2luZ0NvbnRhaW5lckNsaWVudC5zY2FsYQ==) | `70.9% <30%> (ø)` | | | [...whisk/core/containerpool/AkkaContainerClient.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvY29udGFpbmVycG9vbC9Ba2thQ29udGFpbmVyQ2xpZW50LnNjYWxh) | `72.72% <72.72%> (ø)` | | | ... and [9 more](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812?src=pr=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/incubator-openwhisk/pull/3812?src=pr=footer). Last update [e96c1bb...cbeb6d2](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812?src=pr=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
[GitHub] codecov-io edited a comment on issue #3812: ContainerClient + akka http alternative to HttpUtils
codecov-io edited a comment on issue #3812: ContainerClient + akka http alternative to HttpUtils URL: https://github.com/apache/incubator-openwhisk/pull/3812#issuecomment-401212372 # [Codecov](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812?src=pr=h1) Report > Merging [#3812](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-openwhisk/commit/e96c1bbd5a0f54c923f0153116f7c1f165275e94?src=pr=desc) will **decrease** coverage by `4.67%`. > The diff coverage is `54.11%`. [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/graphs/tree.svg?height=150=650=l0YmsiSAso=pr)](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812?src=pr=tree) ```diff @@Coverage Diff @@ ## master#3812 +/- ## == - Coverage 75.78% 71.11% -4.68% == Files 145 146 +1 Lines6897 6954 +57 Branches 418 426 +8 == - Hits 5227 4945 -282 - Misses 1670 2009 +339 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812?src=pr=tree) | Coverage Δ | | |---|---|---| | [...containerpool/kubernetes/KubernetesContainer.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvY29udGFpbmVycG9vbC9rdWJlcm5ldGVzL0t1YmVybmV0ZXNDb250YWluZXIuc2NhbGE=) | `91.66% <ø> (ø)` | :arrow_up: | | [...la/whisk/core/containerpool/ContainerFactory.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvY29udGFpbmVycG9vbC9Db250YWluZXJGYWN0b3J5LnNjYWxh) | `100% <ø> (ø)` | :arrow_up: | | [...la/src/main/scala/whisk/core/mesos/MesosTask.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvbWVzb3MvTWVzb3NUYXNrLnNjYWxh) | `86.11% <ø> (ø)` | :arrow_up: | | [...sk/core/containerpool/docker/DockerContainer.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvY29udGFpbmVycG9vbC9kb2NrZXIvRG9ja2VyQ29udGFpbmVyLnNjYWxh) | `75% <0%> (-2.03%)` | :arrow_down: | | [.../src/main/scala/whisk/http/PoolingRestClient.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2h0dHAvUG9vbGluZ1Jlc3RDbGllbnQuc2NhbGE=) | `90% <100%> (+0.71%)` | :arrow_up: | | [...n/scala/whisk/core/database/CouchDbRestStore.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvQ291Y2hEYlJlc3RTdG9yZS5zY2FsYQ==) | `73.23% <100%> (ø)` | :arrow_up: | | [...on/scala/src/main/scala/whisk/common/Logging.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvbW1vbi9Mb2dnaW5nLnNjYWxh) | `87.09% <100%> (+0.28%)` | :arrow_up: | | [...ain/scala/whisk/core/containerpool/Container.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvY29udGFpbmVycG9vbC9Db250YWluZXIuc2NhbGE=) | `74.24% <20%> (-1.95%)` | :arrow_down: | | [.../containerpool/ApacheBlockingContainerClient.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvY29udGFpbmVycG9vbC9BcGFjaGVCbG9ja2luZ0NvbnRhaW5lckNsaWVudC5zY2FsYQ==) | `70.9% <30%> (ø)` | | | [...whisk/core/containerpool/AkkaContainerClient.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvY29udGFpbmVycG9vbC9Ba2thQ29udGFpbmVyQ2xpZW50LnNjYWxh) | `72.72% <72.72%> (ø)` | | | ... and [9 more](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812?src=pr=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/incubator-openwhisk/pull/3812?src=pr=footer). Last update [e96c1bb...cbeb6d2](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812?src=pr=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
[GitHub] codecov-io edited a comment on issue #3812: ContainerClient + akka http alternative to HttpUtils
codecov-io edited a comment on issue #3812: ContainerClient + akka http alternative to HttpUtils URL: https://github.com/apache/incubator-openwhisk/pull/3812#issuecomment-401212372 # [Codecov](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812?src=pr=h1) Report > Merging [#3812](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-openwhisk/commit/e96c1bbd5a0f54c923f0153116f7c1f165275e94?src=pr=desc) will **decrease** coverage by `4.67%`. > The diff coverage is `54.11%`. [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/graphs/tree.svg?src=pr=l0YmsiSAso=150=650)](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812?src=pr=tree) ```diff @@Coverage Diff @@ ## master#3812 +/- ## == - Coverage 75.78% 71.11% -4.68% == Files 145 146 +1 Lines6897 6954 +57 Branches 418 426 +8 == - Hits 5227 4945 -282 - Misses 1670 2009 +339 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812?src=pr=tree) | Coverage Δ | | |---|---|---| | [...la/src/main/scala/whisk/core/mesos/MesosTask.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvbWVzb3MvTWVzb3NUYXNrLnNjYWxh) | `86.11% <ø> (ø)` | :arrow_up: | | [...la/whisk/core/containerpool/ContainerFactory.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvY29udGFpbmVycG9vbC9Db250YWluZXJGYWN0b3J5LnNjYWxh) | `100% <ø> (ø)` | :arrow_up: | | [...containerpool/kubernetes/KubernetesContainer.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvY29udGFpbmVycG9vbC9rdWJlcm5ldGVzL0t1YmVybmV0ZXNDb250YWluZXIuc2NhbGE=) | `91.66% <ø> (ø)` | :arrow_up: | | [...sk/core/containerpool/docker/DockerContainer.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvY29udGFpbmVycG9vbC9kb2NrZXIvRG9ja2VyQ29udGFpbmVyLnNjYWxh) | `75% <0%> (-2.03%)` | :arrow_down: | | [...n/scala/whisk/core/database/CouchDbRestStore.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvQ291Y2hEYlJlc3RTdG9yZS5zY2FsYQ==) | `73.23% <100%> (ø)` | :arrow_up: | | [...on/scala/src/main/scala/whisk/common/Logging.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvbW1vbi9Mb2dnaW5nLnNjYWxh) | `87.09% <100%> (+0.28%)` | :arrow_up: | | [.../src/main/scala/whisk/http/PoolingRestClient.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2h0dHAvUG9vbGluZ1Jlc3RDbGllbnQuc2NhbGE=) | `90% <100%> (+0.71%)` | :arrow_up: | | [...ain/scala/whisk/core/containerpool/Container.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvY29udGFpbmVycG9vbC9Db250YWluZXIuc2NhbGE=) | `74.24% <20%> (-1.95%)` | :arrow_down: | | [.../containerpool/ApacheBlockingContainerClient.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvY29udGFpbmVycG9vbC9BcGFjaGVCbG9ja2luZ0NvbnRhaW5lckNsaWVudC5zY2FsYQ==) | `70.9% <30%> (ø)` | | | [...whisk/core/containerpool/AkkaContainerClient.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvY29udGFpbmVycG9vbC9Ba2thQ29udGFpbmVyQ2xpZW50LnNjYWxh) | `72.72% <72.72%> (ø)` | | | ... and [9 more](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812?src=pr=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/incubator-openwhisk/pull/3812?src=pr=footer). Last update [e96c1bb...cbeb6d2](https://codecov.io/gh/apache/incubator-openwhisk/pull/3812?src=pr=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
[GitHub] dgrove-oss commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils
dgrove-oss commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils URL: https://github.com/apache/incubator-openwhisk/pull/3812#discussion_r204140363 ## File path: tests/src/test/scala/whisk/core/containerpool/kubernetes/test/KubernetesClientTests.scala ## @@ -188,6 +189,7 @@ object KubernetesClientTests { implicit def strToInstant(str: String): Instant = strToDate(str).get + implicit val as = ActorSystem("kubernetes-client-tests-actor-system") Review comment: I don't remember any particular reason; I think it may be as simple as that since we hadn't needed to have our hands on an ActorSystem before in the stubbed out test client it wasn't plumbed through. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] tysonnorris commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils
tysonnorris commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils URL: https://github.com/apache/incubator-openwhisk/pull/3812#discussion_r204137242 ## File path: tests/src/test/scala/whisk/core/containerpool/kubernetes/test/KubernetesClientTests.scala ## @@ -188,6 +189,7 @@ object KubernetesClientTests { implicit def strToInstant(str: String): Instant = strToDate(str).get + implicit val as = ActorSystem("kubernetes-client-tests-actor-system") Review comment: The `TestKubernetesClient` is shared with `KubernetesContainerTests` - so for now, changed to implicit ActorSystem (and left object/classes in current places). Good? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] houshengbo opened a new pull request #224: Update the download links of the 0.9.0 artifacts with mirror links
houshengbo opened a new pull request #224: Update the download links of the 0.9.0 artifacts with mirror links URL: https://github.com/apache/incubator-openwhisk-release/pull/224 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] tysonnorris commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils
tysonnorris commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils URL: https://github.com/apache/incubator-openwhisk/pull/3812#discussion_r204126416 ## File path: tests/src/test/scala/whisk/core/containerpool/docker/test/AkkaContainerClientTests.scala ## @@ -0,0 +1,213 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package whisk.core.containerpool.docker.test + +import common.StreamLogging +import common.WskActorSystem +import java.nio.charset.StandardCharsets +import java.time.Instant +import org.apache.http.HttpRequest +import org.apache.http.HttpResponse +import org.apache.http.entity.StringEntity +import org.apache.http.localserver.LocalServerTestBase +import org.apache.http.protocol.HttpContext +import org.apache.http.protocol.HttpRequestHandler +import org.junit.runner.RunWith +import org.scalatest.BeforeAndAfter +import org.scalatest.BeforeAndAfterAll +import org.scalatest.FlatSpec +import org.scalatest.Matchers +import org.scalatest.junit.JUnitRunner +import scala.concurrent.Await +import scala.concurrent.TimeoutException +import scala.concurrent.duration._ +import spray.json.JsObject +import whisk.common.TransactionId +import whisk.core.containerpool.AkkaContainerClient +import whisk.core.entity.ActivationResponse._ +import whisk.core.entity.size._ + +/** + * Unit tests for AkkaContainerClientTests which communicate with containers. + */ +@RunWith(classOf[JUnitRunner]) +class AkkaContainerClientTests +extends FlatSpec +with Matchers +with BeforeAndAfter +with BeforeAndAfterAll +with StreamLogging +with WskActorSystem { + + implicit val transid = TransactionId.testing + implicit val ec = actorSystem.dispatcher + + var testHang: FiniteDuration = 0.second + var testStatusCode: Int = 200 + var testResponse: String = null + var testConnectionFailCount: Int = 0 + + val mockServer = new LocalServerTestBase { +var failcount = 0 +override def setUp() = { + super.setUp() + this.serverBootstrap +.registerHandler( + "/init", + new HttpRequestHandler() { +override def handle(request: HttpRequest, response: HttpResponse, context: HttpContext) = { + if (testHang.length > 0) { +Thread.sleep(testHang.toMillis) + } + if (testConnectionFailCount > 0 && failcount < testConnectionFailCount) { +failcount += 1 +println("failing in test") +throw new RuntimeException("failing...") + } + response.setStatusCode(testStatusCode); + if (testResponse != null) { +response.setEntity(new StringEntity(testResponse, StandardCharsets.UTF_8)) + } +} + }) +} + } + + mockServer.setUp() + val httpHost = mockServer.start() + val hostWithPort = s"${httpHost.getHostName}:${httpHost.getPort}" + + before { +testHang = 0.second +testStatusCode = 200 +testResponse = null +testConnectionFailCount = 0 +stream.reset() + } + + override def afterAll = { +mockServer.shutDown() + } + + behavior of "PoolingContainerClient" + + it should "not wait longer than set timeout" in { +val timeout = 5.seconds +val connection = new AkkaContainerClient(httpHost.getHostName, httpHost.getPort, timeout, 1.B, 100) +testHang = timeout * 2 +val start = Instant.now() +val result = Await.result(connection.post("/init", JsObject.empty, retry = true), 10.seconds) + +val end = Instant.now() +val waited = end.toEpochMilli - start.toEpochMilli +result shouldBe 'left +waited should be > timeout.toMillis +waited should be < (timeout * 2).toMillis + } + + it should "handle empty entity response" in { +val timeout = 5.seconds +val connection = new AkkaContainerClient(httpHost.getHostName, httpHost.getPort, timeout, 1.B, 100) +testStatusCode = 204 +val result = Await.result(connection.post("/init", JsObject.empty, retry = true), 10.seconds) +result shouldBe Left(NoResponseReceived()) + } + + it should "retry till timeout on StreamTcpException" in { +val timeout =
[GitHub] tysonnorris commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils
tysonnorris commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils URL: https://github.com/apache/incubator-openwhisk/pull/3812#discussion_r204125651 ## File path: tests/src/test/scala/whisk/core/containerpool/docker/test/AkkaContainerClientTests.scala ## @@ -0,0 +1,213 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package whisk.core.containerpool.docker.test + +import common.StreamLogging +import common.WskActorSystem +import java.nio.charset.StandardCharsets +import java.time.Instant +import org.apache.http.HttpRequest +import org.apache.http.HttpResponse +import org.apache.http.entity.StringEntity +import org.apache.http.localserver.LocalServerTestBase +import org.apache.http.protocol.HttpContext +import org.apache.http.protocol.HttpRequestHandler +import org.junit.runner.RunWith +import org.scalatest.BeforeAndAfter +import org.scalatest.BeforeAndAfterAll +import org.scalatest.FlatSpec +import org.scalatest.Matchers +import org.scalatest.junit.JUnitRunner +import scala.concurrent.Await +import scala.concurrent.TimeoutException +import scala.concurrent.duration._ +import spray.json.JsObject +import whisk.common.TransactionId +import whisk.core.containerpool.AkkaContainerClient +import whisk.core.entity.ActivationResponse._ +import whisk.core.entity.size._ + +/** + * Unit tests for AkkaContainerClientTests which communicate with containers. + */ +@RunWith(classOf[JUnitRunner]) +class AkkaContainerClientTests +extends FlatSpec +with Matchers +with BeforeAndAfter +with BeforeAndAfterAll +with StreamLogging +with WskActorSystem { + + implicit val transid = TransactionId.testing + implicit val ec = actorSystem.dispatcher + + var testHang: FiniteDuration = 0.second + var testStatusCode: Int = 200 + var testResponse: String = null + var testConnectionFailCount: Int = 0 + + val mockServer = new LocalServerTestBase { +var failcount = 0 +override def setUp() = { + super.setUp() + this.serverBootstrap +.registerHandler( + "/init", + new HttpRequestHandler() { +override def handle(request: HttpRequest, response: HttpResponse, context: HttpContext) = { + if (testHang.length > 0) { +Thread.sleep(testHang.toMillis) + } + if (testConnectionFailCount > 0 && failcount < testConnectionFailCount) { +failcount += 1 +println("failing in test") +throw new RuntimeException("failing...") + } + response.setStatusCode(testStatusCode); + if (testResponse != null) { +response.setEntity(new StringEntity(testResponse, StandardCharsets.UTF_8)) + } +} + }) +} + } + + mockServer.setUp() + val httpHost = mockServer.start() + val hostWithPort = s"${httpHost.getHostName}:${httpHost.getPort}" + + before { +testHang = 0.second +testStatusCode = 200 +testResponse = null +testConnectionFailCount = 0 +stream.reset() + } + + override def afterAll = { +mockServer.shutDown() + } + + behavior of "PoolingContainerClient" + + it should "not wait longer than set timeout" in { +val timeout = 5.seconds +val connection = new AkkaContainerClient(httpHost.getHostName, httpHost.getPort, timeout, 1.B, 100) +testHang = timeout * 2 +val start = Instant.now() +val result = Await.result(connection.post("/init", JsObject.empty, retry = true), 10.seconds) + +val end = Instant.now() +val waited = end.toEpochMilli - start.toEpochMilli +result shouldBe 'left +waited should be > timeout.toMillis +waited should be < (timeout * 2).toMillis + } + + it should "handle empty entity response" in { +val timeout = 5.seconds +val connection = new AkkaContainerClient(httpHost.getHostName, httpHost.getPort, timeout, 1.B, 100) +testStatusCode = 204 +val result = Await.result(connection.post("/init", JsObject.empty, retry = true), 10.seconds) +result shouldBe Left(NoResponseReceived()) + } + + it should "retry till timeout on StreamTcpException" in { +val timeout =
[GitHub] tysonnorris commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils
tysonnorris commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils URL: https://github.com/apache/incubator-openwhisk/pull/3812#discussion_r204125226 ## File path: tests/src/test/scala/whisk/core/containerpool/docker/test/ApacheBlockingContainerClientTests.scala ## @@ -106,20 +113,41 @@ class ContainerConnectionTests it should "handle empty entity response" in { val timeout = 5.seconds -val connection = new HttpUtils(hostWithPort, timeout, 1.B) +val connection = new ApacheBlockingContainerClient(hostWithPort, timeout, 1.B) testStatusCode = 204 -val result = connection.post("/init", JsObject.empty, retry = true) +val result = Await.result(connection.post("/init", JsObject.empty, retry = true), 10.seconds) result shouldBe Left(NoResponseReceived()) } + it should "retry till timeout on HttpHostConnectException" in { +val timeout = 5.seconds +val badHostAndPort = "0.0.0.0:12345" +val connection = new ApacheBlockingContainerClient(badHostAndPort, timeout, 1.B) +testStatusCode = 204 +val start = Instant.now() +val result = Await.result(connection.post("/init", JsObject.empty, retry = true), 10.seconds) +val end = Instant.now() +val waited = end.toEpochMilli - start.toEpochMilli +result should be('left) +result.left.get shouldBe a[Timeout] +result.left.get.asInstanceOf[Timeout].t shouldBe a[RetryableConnectionError] +result.left.get + .asInstanceOf[Timeout] + .t + .asInstanceOf[RetryableConnectionError] + .t shouldBe a[HttpHostConnectException] Review comment: nice - much better! (worked for me) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] tysonnorris commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils
tysonnorris commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils URL: https://github.com/apache/incubator-openwhisk/pull/3812#discussion_r204123577 ## File path: tests/src/test/scala/whisk/core/containerpool/kubernetes/test/KubernetesClientTests.scala ## @@ -188,6 +189,7 @@ object KubernetesClientTests { implicit def strToInstant(str: String): Instant = strToDate(str).get + implicit val as = ActorSystem("kubernetes-client-tests-actor-system") Review comment: I will give it a try, but it isn't clear why this test was setup this way? @dgrove-oss @jcrossley3 ? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] tysonnorris commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils
tysonnorris commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils URL: https://github.com/apache/incubator-openwhisk/pull/3812#discussion_r204115351 ## File path: core/invoker/src/main/scala/whisk/core/containerpool/docker/DockerContainer.scala ## @@ -190,29 +190,33 @@ class DockerContainer(protected val id: ContainerId, implicit transid: TransactionId): Future[RunResult] = { val started = Instant.now() val http = httpConnection.getOrElse { - val conn = new HttpUtils(s"${addr.host}:${addr.port}", timeout, ActivationEntityLimit.MAX_ACTIVATION_ENTITY_LIMIT) + val conn = new ApacheBlockingContainerClient( +s"${addr.host}:${addr.port}", +timeout, +ActivationEntityLimit.MAX_ACTIVATION_ENTITY_LIMIT) Review comment: Also noticed that `ActivationEntityLimit.MAX_ACTIVATION_ENTITY_LIMIT` is not used in `Container.scala`... These are both 1mb, but docs for `MAX_ACTIVATION_LIMIT` says _This refers to the invoke-time parameters_ - but in this case we are limiting the response size (and I don't see any assertion of limit on the request entity size in former HttpUtils?). WDYT? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] tysonnorris commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils
tysonnorris commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils URL: https://github.com/apache/incubator-openwhisk/pull/3812#discussion_r204112297 ## File path: common/scala/src/main/scala/whisk/core/containerpool/AkkaContainerClient.scala ## @@ -0,0 +1,216 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package whisk.core.containerpool + +import akka.actor.ActorSystem +import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport._ +import akka.http.scaladsl.marshalling.Marshal +import akka.http.scaladsl.model.HttpMethods +import akka.http.scaladsl.model.HttpRequest +import akka.http.scaladsl.model.HttpResponse +import akka.http.scaladsl.model.MediaTypes +import akka.http.scaladsl.model.MessageEntity +import akka.http.scaladsl.model.StatusCodes +import akka.http.scaladsl.model.headers.Accept +import akka.http.scaladsl.model.headers.Connection +import akka.http.scaladsl.unmarshalling.Unmarshal +import akka.stream.StreamTcpException +import akka.stream.scaladsl.Sink +import akka.stream.scaladsl.Source +import akka.util.ByteString +import scala.concurrent.Await +import scala.concurrent.ExecutionContext +import scala.concurrent.Future +import scala.concurrent.TimeoutException +import scala.concurrent.duration._ +import scala.util.Try +import scala.util.control.NonFatal +import spray.json._ +import whisk.common.Logging +import whisk.common.TransactionId +import whisk.core.entity.ActivationResponse.ContainerHttpError +import whisk.core.entity.ActivationResponse._ +import whisk.core.entity.ByteSize +import whisk.core.entity.size.SizeLong +import whisk.http.PoolingRestClient + +/** + * This HTTP client is used only in the invoker to communicate with the action container. + * It allows to POST a JSON object and receive JSON object back; that is the + * content type and the accept headers are both 'application/json. + * This implementation uses the akka http host-level client API. + * + * @param hostname the host name + * @param port the port + * @param timeout the timeout in msecs to wait for a response + * @param maxResponse the maximum size in bytes the connection will accept + * @param queueSize once all connections are used, how big of queue to allow for additional requests + * @param retryInterval duration between retries for TCP connection errors + */ +protected class AkkaContainerClient( + hostname: String, + port: Int, + timeout: FiniteDuration, + maxResponse: ByteSize, + queueSize: Int, + retryInterval: FiniteDuration = 100.milliseconds)(implicit logging: Logging, as: ActorSystem) +extends PoolingRestClient("http", hostname, port, queueSize, timeout = Some(timeout)) +with ContainerClient { + + def close() = Await.result(shutdown(), 30.seconds) + + /** + * Posts to hostname/endpoint the given JSON object. + * Waits up to timeout before aborting on a good connection. + * If the endpoint is not ready, retry up to timeout. + * Every retry reduces the available timeout so that this method should not + * wait longer than the total timeout (within a small slack allowance). + * + * @param endpoint the path the api call relative to hostname + * @param body the JSON value to post (this is usually a JSON objecT) + * @param retry whether or not to retry on connection failure + * @return Left(Error Message) or Right(Status Code, Response as UTF-8 String) + */ + def post(endpoint: String, body: JsValue, retry: Boolean)( +implicit tid: TransactionId): Future[Either[ContainerHttpError, ContainerResponse]] = { + +//create the request +val req = Marshal(body).to[MessageEntity].map { b => + //DO NOT reuse the connection + //For details on Connection: Close handling, see: + // - https://doc.akka.io/docs/akka-http/current/common/http-model.html#http-headers + // - http://github.com/akka/akka-http/tree/v10.1.3/akka-http-core/src/test/scala/akka/http/impl/engine/rendering/ResponseRendererSpec.scala#L470-L571 + HttpRequest(HttpMethods.POST, endpoint, entity = b) +.withHeaders(Connection("close"), Accept(MediaTypes.`application/json`)) +} + +retryingRequest(req, timeout, retry) + .flatMap
[GitHub] vvraskin commented on issue #3853: Set erlang magic cookie for couchdb
vvraskin commented on issue #3853: Set erlang magic cookie for couchdb URL: https://github.com/apache/incubator-openwhisk/pull/3853#issuecomment-406654701 Or should the cookie be always unique? What if we don't generate it each time when couchdb is deployed and use a static property instead? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] vvraskin commented on issue #3853: Set erlang magic cookie for couchdb
vvraskin commented on issue #3853: Set erlang magic cookie for couchdb URL: https://github.com/apache/incubator-openwhisk/pull/3853#issuecomment-406653353 I've just tested it on macOS, unfortunately it breaks up on the step when the cookie is generated. ``` TASK [couchdb : create the erlang cookie file on remote] *** Friday 20 July 2018 18:20:28 +0200 (0:00:00.451) 0:00:03.203 *** fatal: [172.17.0.1]: FAILED! => {"changed": true, "cmd": ["docker", "run", "--rm", "-v", "/tmp:/tmp", "-u", "couchdb", "apache/couchdb:2.1", "sh", "-c", "echo rBA/m2eUwv7FML/UhasE2KdPoUkr6iahNx7x6UQt+6c= >> /tmp/erlang.cookie"], "delta": "0:00:01.200721", "end": "2018-07-20 18:20:30.445213", "msg": "non-zero return code", "rc": 2, "start": "2018-07-20 18:20:29.244492", "stderr": "sh: 1: cannot create /tmp/erlang.cookie: Permission denied", "stderr_lines": ["sh: 1: cannot create /tmp/erlang.cookie: Permission denied"], "stdout": "", "stdout_lines": []} ``` I think that the problem relates to the fact that `/tmp` is owned by root on macOS and the way how it is mounted in docker (not sure how it is exactly done there). I've tried to escalate the permissions with `become:true` for that task, but it failed as well prompting for sudo password. Is there a way to generate the cookie as a local action and then change the owner to couchdb? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] codecov-io edited a comment on issue #3887: Implement full specification of docker image names.
codecov-io edited a comment on issue #3887: Implement full specification of docker image names. URL: https://github.com/apache/incubator-openwhisk/pull/3887#issuecomment-406617232 # [Codecov](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887?src=pr=h1) Report > Merging [#3887](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-openwhisk/commit/1b8ffcd3203b38eae045a915211ad6d84cc72638?src=pr=desc) will **decrease** coverage by `4.69%`. > The diff coverage is `100%`. [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887/graphs/tree.svg?src=pr=l0YmsiSAso=650=150)](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887?src=pr=tree) ```diff @@Coverage Diff@@ ## master#3887 +/- ## = - Coverage 75.72% 71.02% -4.7% = Files 145 145 Lines6901 6914 +13 Branches 417 414 -3 = - Hits 5226 4911-315 - Misses 1675 2003+328 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887?src=pr=tree) | Coverage Δ | | |---|---|---| | [...rc/main/scala/whisk/core/entity/ExecManifest.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZW50aXR5L0V4ZWNNYW5pZmVzdC5zY2FsYQ==) | `97.87% <100%> (+1.57%)` | :arrow_up: | | [...core/database/cosmosdb/RxObservableImplicits.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvUnhPYnNlcnZhYmxlSW1wbGljaXRzLnNjYWxh) | `0% <0%> (-100%)` | :arrow_down: | | [...core/database/cosmosdb/CosmosDBArtifactStore.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlLnNjYWxh) | `0% <0%> (-95.08%)` | :arrow_down: | | [...sk/core/database/cosmosdb/CosmosDBViewMapper.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJWaWV3TWFwcGVyLnNjYWxh) | `0% <0%> (-92.6%)` | :arrow_down: | | [...whisk/core/database/cosmosdb/CosmosDBSupport.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJTdXBwb3J0LnNjYWxh) | `0% <0%> (-81.82%)` | :arrow_down: | | [...abase/cosmosdb/CosmosDBArtifactStoreProvider.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlUHJvdmlkZXIuc2NhbGE=) | `0% <0%> (-58.83%)` | :arrow_down: | | [...la/whisk/core/database/cosmosdb/CosmosDBUtil.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJVdGlsLnNjYWxh) | `92% <0%> (-4%)` | :arrow_down: | | [...rc/main/scala/whisk/common/ForcableSemaphore.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvbW1vbi9Gb3JjYWJsZVNlbWFwaG9yZS5zY2FsYQ==) | `88.46% <0%> (+3.84%)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887?src=pr=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/incubator-openwhisk/pull/3887?src=pr=footer). Last update [1b8ffcd...23caa41](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887?src=pr=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rabbah closed pull request #261: Move db initialization into separate Job and reorder invoker init-containers
rabbah closed pull request #261: Move db initialization into separate Job and reorder invoker init-containers URL: https://github.com/apache/incubator-openwhisk-deploy-kube/pull/261 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/docker/README.md b/docker/README.md index c0915bc..22a2db9 100644 --- a/docker/README.md +++ b/docker/README.md @@ -28,8 +28,5 @@ The built images are: ansible playbooks. * whisk-script-runner - An alpine-based utility image for running simple bash scripts that need the `wsk` cli available to them. - * couchdb - creates and initializes a CouchDB instance for -dev/testing of OpenWhisk. This image is not intended for -production usage. * invoker-agent - worker node invoker agent -- used to implement suspend/resume and log consolidation ops for a remote invoker diff --git a/docker/couchdb/Dockerfile b/docker/couchdb/Dockerfile deleted file mode 100644 index 3c58a22..000 --- a/docker/couchdb/Dockerfile +++ /dev/null @@ -1,31 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one or more contributor -# license agreements; and to You under the Apache License, Version 2.0. - -FROM apache/couchdb:2.1 - -ADD http://ftp.us.debian.org/debian/pool/main/a/apt/apt-transport-https_1.0.9.8.4_amd64.deb /tmp/apt-transport-https_1.0.9.8.4_amd64.deb -ADD http://ftp.us.debian.org/debian/pool/main/c/curl/libcurl3-gnutls_7.38.0-4+deb8u8_amd64.deb /tmp/libcurl3-gnutls_7.38.0-4+deb8u58_amd64.deb -RUN dpkg -i /tmp/libcurl3-gnutls_7.38.0-4+deb8u58_amd64.deb -RUN dpkg -i /tmp/apt-transport-https_1.0.9.8.4_amd64.deb - -RUN apt-get -y update && apt-get -y install \ - git \ - curl \ - sudo \ - python-dev \ - python-pip \ - libffi-dev \ - libssl-dev - -RUN pip install --upgrade setuptools -RUN pip install argcomplete -RUN pip install couchdb -RUN pip install --upgrade cffi -RUN pip install markupsafe -RUN pip install ansible==2.5.2 -RUN pip install -U pyopenssl - -COPY init.sh /init.sh -RUN chmod +X /init.sh - -CMD ["/init.sh"] diff --git a/docker/couchdb/init.sh b/docker/couchdb/init.sh deleted file mode 100755 index fe0a64f..000 --- a/docker/couchdb/init.sh +++ /dev/null @@ -1,94 +0,0 @@ -#!/bin/bash -# Licensed to the Apache Software Foundation (ASF) under one or more contributor -# license agreements; and to You under the Apache License, Version 2.0. - -set -ex - -# start couchdb as a background process -/docker-entrypoint.sh /opt/couchdb/bin/couchdb & - -# wait for couchdb to be up and running -TIMEOUT=0 -echo "wait for CouchDB to be up and running" -until $( curl --output /dev/null --silent http://$DB_HOST:$DB_PORT/_utils ) || [ $TIMEOUT -eq 30 ]; do -echo "waiting for CouchDB to be available" - -sleep 2 -let TIMEOUT=TIMEOUT+1 -done - -if [ $TIMEOUT -eq 30 ]; then -echo "failed to setup CouchDB" -exit 1 -fi - - -if [[ -f /opt/couchdb/data/_openwhisk_initialized.stamp ]]; then -# If we mounted an existing database from the persistent volume then use it. -echo "_openwhisk_initialized.stamp exists; using existing database" -else -# No existing database; we will have to initialize it. - -# Always clone the latest version of OpenWhisk -git clone https://github.com/apache/incubator-openwhisk /openwhisk - -pushd /openwhisk -# if auth guest overwrite file -if [ -n "$AUTH_GUEST" ]; then -echo "$AUTH_GUEST" > /openwhisk/ansible/files/auth.guest -fi - -# if auth whisk system overwrite file -if [ -n "$AUTH_WHISK_SYSTEM" ]; then -echo "$AUTH_WHISK_SYSTEM" > /openwhisk/ansible/files/auth.whisk.system -fi - -# setup and initialize DB -pushd ansible -ansible-playbook -i environments/local setup.yml -ansible-playbook -i environments/local couchdb.yml --tags ini \ - -e db_prefix=$DB_PREFIX \ - -e db_host=$DB_HOST \ - -e db_username=$COUCHDB_USER \ - -e db_password=$COUCHDB_PASSWORD \ - -e db_port=$DB_PORT \ - -e openwhisk_home=/openwhisk -popd - -# disable reduce limits on views -curl -X PUT http://$COUCHDB_USER:$COUCHDB_PASSWORD@$DB_HOST:$DB_PORT/_node/couchdb@$NODENAME/_config/query_server_config/reduce_limit -d '"false"' - -# create the couchdb system databases -curl -X PUT http://$COUCHDB_USER:$COUCHDB_PASSWORD@$DB_HOST:$DB_PORT/_users -curl -X PUT http://$COUCHDB_USER:$COUCHDB_PASSWORD@$DB_HOST:$DB_PORT/_replicator -curl -X PUT http://$COUCHDB_USER:$COUCHDB_PASSWORD@$DB_HOST:$DB_PORT/_global_changes - -pushd ansible -
[GitHub] rabbah closed issue #252: Adding Ressource libcurl3-gnutls in couchDB Dockerfile uses deprecated link
rabbah closed issue #252: Adding Ressource libcurl3-gnutls in couchDB Dockerfile uses deprecated link URL: https://github.com/apache/incubator-openwhisk-deploy-kube/issues/252 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] codecov-io edited a comment on issue #3887: Implement full specification of docker image names.
codecov-io edited a comment on issue #3887: Implement full specification of docker image names. URL: https://github.com/apache/incubator-openwhisk/pull/3887#issuecomment-406617232 # [Codecov](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887?src=pr=h1) Report > Merging [#3887](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-openwhisk/commit/1b8ffcd3203b38eae045a915211ad6d84cc72638?src=pr=desc) will **decrease** coverage by `5.5%`. > The diff coverage is `100%`. [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887/graphs/tree.svg?src=pr=l0YmsiSAso=650=150)](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887?src=pr=tree) ```diff @@Coverage Diff @@ ## master#3887 +/- ## == - Coverage 75.72% 70.21% -5.51% == Files 145 145 Lines6901 6914 +13 Branches 417 414 -3 == - Hits 5226 4855 -371 - Misses 1675 2059 +384 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887?src=pr=tree) | Coverage Δ | | |---|---|---| | [...rc/main/scala/whisk/core/entity/ExecManifest.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZW50aXR5L0V4ZWNNYW5pZmVzdC5zY2FsYQ==) | `85.1% <100%> (-11.19%)` | :arrow_down: | | [...core/database/cosmosdb/RxObservableImplicits.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvUnhPYnNlcnZhYmxlSW1wbGljaXRzLnNjYWxh) | `0% <0%> (-100%)` | :arrow_down: | | [...core/database/cosmosdb/CosmosDBArtifactStore.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlLnNjYWxh) | `0% <0%> (-95.08%)` | :arrow_down: | | [...sk/core/database/cosmosdb/CosmosDBViewMapper.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJWaWV3TWFwcGVyLnNjYWxh) | `0% <0%> (-92.6%)` | :arrow_down: | | [...whisk/core/database/cosmosdb/CosmosDBSupport.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJTdXBwb3J0LnNjYWxh) | `0% <0%> (-81.82%)` | :arrow_down: | | [...abase/cosmosdb/CosmosDBArtifactStoreProvider.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlUHJvdmlkZXIuc2NhbGE=) | `0% <0%> (-58.83%)` | :arrow_down: | | [...whisk/connector/kafka/KafkaMessagingProvider.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2Nvbm5lY3Rvci9rYWZrYS9LYWZrYU1lc3NhZ2luZ1Byb3ZpZGVyLnNjYWxh) | `23.07% <0%> (-46.16%)` | :arrow_down: | | [...whisk/connector/kafka/KafkaProducerConnector.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2Nvbm5lY3Rvci9rYWZrYS9LYWZrYVByb2R1Y2VyQ29ubmVjdG9yLnNjYWxh) | `22.22% <0%> (-36.12%)` | :arrow_down: | | [...whisk/connector/kafka/KafkaConsumerConnector.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2Nvbm5lY3Rvci9rYWZrYS9LYWZrYUNvbnN1bWVyQ29ubmVjdG9yLnNjYWxh) | `33.33% <0%> (-29.63%)` | :arrow_down: | | ... and [5 more](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887?src=pr=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/incubator-openwhisk/pull/3887?src=pr=footer). Last update [1b8ffcd...23caa41](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887?src=pr=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to
[GitHub] codecov-io commented on issue #3887: Implement full specification of docker image names.
codecov-io commented on issue #3887: Implement full specification of docker image names. URL: https://github.com/apache/incubator-openwhisk/pull/3887#issuecomment-406617232 # [Codecov](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887?src=pr=h1) Report > Merging [#3887](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-openwhisk/commit/1b8ffcd3203b38eae045a915211ad6d84cc72638?src=pr=desc) will **decrease** coverage by `4.7%`. > The diff coverage is `100%`. [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887/graphs/tree.svg?width=650=150=pr=l0YmsiSAso)](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887?src=pr=tree) ```diff @@Coverage Diff @@ ## master#3887 +/- ## == - Coverage 75.72% 71.02% -4.71% == Files 145 145 Lines6901 6913 +12 Branches 417 409 -8 == - Hits 5226 4910 -316 - Misses 1675 2003 +328 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887?src=pr=tree) | Coverage Δ | | |---|---|---| | [...rc/main/scala/whisk/core/entity/ExecManifest.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZW50aXR5L0V4ZWNNYW5pZmVzdC5zY2FsYQ==) | `97.84% <100%> (+1.55%)` | :arrow_up: | | [...core/database/cosmosdb/RxObservableImplicits.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvUnhPYnNlcnZhYmxlSW1wbGljaXRzLnNjYWxh) | `0% <0%> (-100%)` | :arrow_down: | | [...core/database/cosmosdb/CosmosDBArtifactStore.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlLnNjYWxh) | `0% <0%> (-95.08%)` | :arrow_down: | | [...sk/core/database/cosmosdb/CosmosDBViewMapper.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJWaWV3TWFwcGVyLnNjYWxh) | `0% <0%> (-92.6%)` | :arrow_down: | | [...whisk/core/database/cosmosdb/CosmosDBSupport.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJTdXBwb3J0LnNjYWxh) | `0% <0%> (-81.82%)` | :arrow_down: | | [...abase/cosmosdb/CosmosDBArtifactStoreProvider.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlUHJvdmlkZXIuc2NhbGE=) | `0% <0%> (-58.83%)` | :arrow_down: | | [...la/whisk/core/database/cosmosdb/CosmosDBUtil.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJVdGlsLnNjYWxh) | `92% <0%> (-4%)` | :arrow_down: | | [...rc/main/scala/whisk/common/ForcableSemaphore.scala](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL3doaXNrL2NvbW1vbi9Gb3JjYWJsZVNlbWFwaG9yZS5zY2FsYQ==) | `88.46% <0%> (+3.84%)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887?src=pr=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/incubator-openwhisk/pull/3887?src=pr=footer). Last update [1b8ffcd...07358c0](https://codecov.io/gh/apache/incubator-openwhisk/pull/3887?src=pr=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] markusthoemmes opened a new pull request #3887: Implement full specification of docker image names.
markusthoemmes opened a new pull request #3887: Implement full specification of docker image names. URL: https://github.com/apache/incubator-openwhisk/pull/3887 Today we have a very basic way of parsing docker image names, which even prohibit the use of ports in the domain names and/or providing a fully-qualified digest to pull. This parser implements the full specification of docker image names to allow OpenWhisk to take all kinds of image references possible. ## My changes affect the following components - [X] 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 - [X] 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: - [X] I signed an [Apache CLA](https://github.com/apache/incubator-openwhisk/blob/master/CONTRIBUTING.md). - [X] I reviewed the [style guides](https://github.com/apache/incubator-openwhisk/wiki/Contributing:-Git-guidelines#code-readiness) and followed the recommendations (Travis CI will check :). - [X] 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 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] chetanmeh commented on issue #3812: ContainerClient + akka http alternative to HttpUtils
chetanmeh commented on issue #3812: ContainerClient + akka http alternative to HttpUtils URL: https://github.com/apache/incubator-openwhisk/pull/3812#issuecomment-406583458 > One anomaly in the PR conversation, I'm not sure where this is coming from? @tysonnorris Yeh that is a confusing part. This happens because on Master builds CosmosDB test run properly but for PR runs they do not run. Hence you would see codecov showing drop in coverage for each PR. Do not have a good solution for it. One way may be to skip coverage calculation for such code paths This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] markusthoemmes commented on issue #3619: Provide an activation store SPI
markusthoemmes commented on issue #3619: Provide an activation store SPI URL: https://github.com/apache/incubator-openwhisk/pull/3619#issuecomment-406566584 @ningyougang sure it makes sense. @dubee is working on it already though if I'm not mistaken, hence I'd like his input here. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ningyougang edited a comment on issue #3619: Provide an activation store SPI
ningyougang edited a comment on issue #3619: Provide an activation store SPI URL: https://github.com/apache/incubator-openwhisk/pull/3619#issuecomment-406547593 ### 1. Why need to write a new ActivationStoreProvider for `elasticsearch`? Currently, `activations` are saved into `couchdb` using `ArtifactActivationStore` couchdb has below disadvantage for saving activations * It is not convenience to `delete big activations`(activation data is growing very fast on production env, when disk is full, need to delete, but should do compression at first, otherwise, disk usage will increase) * Couchdb's search feature is not stronger than `elasticsearch`. * Maybe users has requirement to query user logs or activation in page, if we use `elasticsearch`, we can use `ELK`(elasticsearch, logstash, kibana) solution to give user power query function. It seems `IBM cloud` also use `kibana` for `log` tab. Since we provide `Provide an activation store SPI` in this patch, so i think it is necessary to implement a new ActivationStoreProvider for `elasticsearch` ### 2.How to implement it? * Implement this trait `ActivationStore`, for example: ``` class ElasticsearchActivationStore(actorSystem: ActorSystem, actorMaterializer: ActorMaterializer, logging: Logging) extends ActivationStore ``` * In class ElasticsearchActivationStore inner, we can use `https://github.com/sksamuel/elastic4s/` to implement it. @markusthoemmes ,how do you think? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ningyougang edited a comment on issue #3619: Provide an activation store SPI
ningyougang edited a comment on issue #3619: Provide an activation store SPI URL: https://github.com/apache/incubator-openwhisk/pull/3619#issuecomment-406547593 ### 1. Why need to write a new ActivationStoreProvider for `elasticsearch`? Currently, `activations` are saved into `couchdb` using `ArtifactActivationStore` couchdb has below disadvantage for saving activations * It is not convenience to `delete big activations`(activation data is growing very fast on production env, when disk is full, need to delete, but should do compression at first, otherwise, disk usage will increase) * Couchdb's search feature is not stronger than `elasticsearch`. Since we provide `Provide an activation store SPI` in this patch, so i think it is necessary to implement a new ActivationStoreProvider for `elasticsearch` ### 2.How to implement it? * Implement this trait `ActivationStore`, for example: ``` class ElasticsearchActivationStore(actorSystem: ActorSystem, actorMaterializer: ActorMaterializer, logging: Logging) extends ActivationStore ``` * In class ElasticsearchActivationStore inner, we can use `https://github.com/sksamuel/elastic4s/` to implement it. @markusthoemmes ,how do you think? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ningyougang edited a comment on issue #3619: Provide an activation store SPI
ningyougang edited a comment on issue #3619: Provide an activation store SPI URL: https://github.com/apache/incubator-openwhisk/pull/3619#issuecomment-406547593 ### 1. Why need to write a new ActivationStoreProvider for `elasticsearch`? Currently, `activations` are saved into `couchdb` using `ArtifactActivationStore` couchdb has below disadvantage for saving activations * It is not convenience to `delete big activations`(activation data is growing very fast for production env, when disk is full, need to delete, but should do compression at first, otherwise, disk usage will increase) * Couchdb's search feature is not stronger than `elasticsearch`. Since we provide `Provide an activation store SPI` in this patch, so i think it is necessary to implement a new ActivationStoreProvider for `elasticsearch` ### 2.How to implement it? * Implement this trait `ActivationStore`, for example: ``` class ElasticsearchActivationStore(actorSystem: ActorSystem, actorMaterializer: ActorMaterializer, logging: Logging) extends ActivationStore ``` * In class ElasticsearchActivationStore inner, we can use `https://github.com/sksamuel/elastic4s/` to implement it. @markusthoemmes ,how do you think? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] markusthoemmes commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils
markusthoemmes commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils URL: https://github.com/apache/incubator-openwhisk/pull/3812#discussion_r203980538 ## File path: core/invoker/src/main/scala/whisk/core/containerpool/docker/DockerContainer.scala ## @@ -190,29 +190,33 @@ class DockerContainer(protected val id: ContainerId, implicit transid: TransactionId): Future[RunResult] = { val started = Instant.now() val http = httpConnection.getOrElse { - val conn = new HttpUtils(s"${addr.host}:${addr.port}", timeout, ActivationEntityLimit.MAX_ACTIVATION_ENTITY_LIMIT) + val conn = new ApacheBlockingContainerClient( +s"${addr.host}:${addr.port}", +timeout, +ActivationEntityLimit.MAX_ACTIVATION_ENTITY_LIMIT) Review comment: Shouldn't this also check the feature toggle and use the correct client as configured? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] markusthoemmes commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils
markusthoemmes commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils URL: https://github.com/apache/incubator-openwhisk/pull/3812#discussion_r203974989 ## File path: common/scala/src/main/scala/whisk/core/containerpool/AkkaContainerClient.scala ## @@ -0,0 +1,216 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package whisk.core.containerpool + +import akka.actor.ActorSystem +import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport._ +import akka.http.scaladsl.marshalling.Marshal +import akka.http.scaladsl.model.HttpMethods +import akka.http.scaladsl.model.HttpRequest +import akka.http.scaladsl.model.HttpResponse +import akka.http.scaladsl.model.MediaTypes +import akka.http.scaladsl.model.MessageEntity +import akka.http.scaladsl.model.StatusCodes +import akka.http.scaladsl.model.headers.Accept +import akka.http.scaladsl.model.headers.Connection +import akka.http.scaladsl.unmarshalling.Unmarshal +import akka.stream.StreamTcpException +import akka.stream.scaladsl.Sink +import akka.stream.scaladsl.Source +import akka.util.ByteString +import scala.concurrent.Await +import scala.concurrent.ExecutionContext +import scala.concurrent.Future +import scala.concurrent.TimeoutException +import scala.concurrent.duration._ +import scala.util.Try +import scala.util.control.NonFatal +import spray.json._ +import whisk.common.Logging +import whisk.common.TransactionId +import whisk.core.entity.ActivationResponse.ContainerHttpError +import whisk.core.entity.ActivationResponse._ +import whisk.core.entity.ByteSize +import whisk.core.entity.size.SizeLong +import whisk.http.PoolingRestClient + +/** + * This HTTP client is used only in the invoker to communicate with the action container. + * It allows to POST a JSON object and receive JSON object back; that is the + * content type and the accept headers are both 'application/json. + * This implementation uses the akka http host-level client API. + * + * @param hostname the host name + * @param port the port + * @param timeout the timeout in msecs to wait for a response + * @param maxResponse the maximum size in bytes the connection will accept + * @param queueSize once all connections are used, how big of queue to allow for additional requests + * @param retryInterval duration between retries for TCP connection errors + */ +protected class AkkaContainerClient( + hostname: String, + port: Int, + timeout: FiniteDuration, + maxResponse: ByteSize, + queueSize: Int, + retryInterval: FiniteDuration = 100.milliseconds)(implicit logging: Logging, as: ActorSystem) +extends PoolingRestClient("http", hostname, port, queueSize, timeout = Some(timeout)) +with ContainerClient { + + def close() = Await.result(shutdown(), 30.seconds) + + /** + * Posts to hostname/endpoint the given JSON object. + * Waits up to timeout before aborting on a good connection. + * If the endpoint is not ready, retry up to timeout. + * Every retry reduces the available timeout so that this method should not + * wait longer than the total timeout (within a small slack allowance). + * + * @param endpoint the path the api call relative to hostname + * @param body the JSON value to post (this is usually a JSON objecT) + * @param retry whether or not to retry on connection failure + * @return Left(Error Message) or Right(Status Code, Response as UTF-8 String) + */ + def post(endpoint: String, body: JsValue, retry: Boolean)( +implicit tid: TransactionId): Future[Either[ContainerHttpError, ContainerResponse]] = { + +//create the request +val req = Marshal(body).to[MessageEntity].map { b => + //DO NOT reuse the connection + //For details on Connection: Close handling, see: + // - https://doc.akka.io/docs/akka-http/current/common/http-model.html#http-headers + // - http://github.com/akka/akka-http/tree/v10.1.3/akka-http-core/src/test/scala/akka/http/impl/engine/rendering/ResponseRendererSpec.scala#L470-L571 + HttpRequest(HttpMethods.POST, endpoint, entity = b) +.withHeaders(Connection("close"), Accept(MediaTypes.`application/json`)) +} + +retryingRequest(req, timeout, retry) +
[GitHub] markusthoemmes commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils
markusthoemmes commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils URL: https://github.com/apache/incubator-openwhisk/pull/3812#discussion_r203989970 ## File path: tests/src/test/scala/whisk/core/containerpool/docker/test/ApacheBlockingContainerClientTests.scala ## @@ -106,20 +113,41 @@ class ContainerConnectionTests it should "handle empty entity response" in { val timeout = 5.seconds -val connection = new HttpUtils(hostWithPort, timeout, 1.B) +val connection = new ApacheBlockingContainerClient(hostWithPort, timeout, 1.B) testStatusCode = 204 -val result = connection.post("/init", JsObject.empty, retry = true) +val result = Await.result(connection.post("/init", JsObject.empty, retry = true), 10.seconds) result shouldBe Left(NoResponseReceived()) } + it should "retry till timeout on HttpHostConnectException" in { +val timeout = 5.seconds +val badHostAndPort = "0.0.0.0:12345" +val connection = new ApacheBlockingContainerClient(badHostAndPort, timeout, 1.B) +testStatusCode = 204 +val start = Instant.now() +val result = Await.result(connection.post("/init", JsObject.empty, retry = true), 10.seconds) +val end = Instant.now() +val waited = end.toEpochMilli - start.toEpochMilli +result should be('left) +result.left.get shouldBe a[Timeout] +result.left.get.asInstanceOf[Timeout].t shouldBe a[RetryableConnectionError] +result.left.get + .asInstanceOf[Timeout] + .t + .asInstanceOf[RetryableConnectionError] + .t shouldBe a[HttpHostConnectException] Review comment: This could be rewritten a little less verbose like: ```scala result match { case Left(Timeout(RetryableConnectionError(_: HttpHostConnectException))) => // all good case _ => fail(s"$result was not a Timeout(RetryableConnectionError(HttpHostConnectException)))") } ``` I was not able to reproduce this locally though, the test always failed with a ConnectError in both implementations. Is this intermittent? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] markusthoemmes commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils
markusthoemmes commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils URL: https://github.com/apache/incubator-openwhisk/pull/3812#discussion_r203991442 ## File path: tests/src/test/scala/whisk/core/containerpool/docker/test/AkkaContainerClientTests.scala ## @@ -0,0 +1,213 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package whisk.core.containerpool.docker.test + +import common.StreamLogging +import common.WskActorSystem +import java.nio.charset.StandardCharsets +import java.time.Instant +import org.apache.http.HttpRequest +import org.apache.http.HttpResponse +import org.apache.http.entity.StringEntity +import org.apache.http.localserver.LocalServerTestBase +import org.apache.http.protocol.HttpContext +import org.apache.http.protocol.HttpRequestHandler +import org.junit.runner.RunWith +import org.scalatest.BeforeAndAfter +import org.scalatest.BeforeAndAfterAll +import org.scalatest.FlatSpec +import org.scalatest.Matchers +import org.scalatest.junit.JUnitRunner +import scala.concurrent.Await +import scala.concurrent.TimeoutException +import scala.concurrent.duration._ +import spray.json.JsObject +import whisk.common.TransactionId +import whisk.core.containerpool.AkkaContainerClient +import whisk.core.entity.ActivationResponse._ +import whisk.core.entity.size._ + +/** + * Unit tests for AkkaContainerClientTests which communicate with containers. + */ +@RunWith(classOf[JUnitRunner]) +class AkkaContainerClientTests +extends FlatSpec +with Matchers +with BeforeAndAfter +with BeforeAndAfterAll +with StreamLogging +with WskActorSystem { + + implicit val transid = TransactionId.testing + implicit val ec = actorSystem.dispatcher + + var testHang: FiniteDuration = 0.second + var testStatusCode: Int = 200 + var testResponse: String = null + var testConnectionFailCount: Int = 0 + + val mockServer = new LocalServerTestBase { +var failcount = 0 +override def setUp() = { + super.setUp() + this.serverBootstrap +.registerHandler( + "/init", + new HttpRequestHandler() { +override def handle(request: HttpRequest, response: HttpResponse, context: HttpContext) = { + if (testHang.length > 0) { +Thread.sleep(testHang.toMillis) + } + if (testConnectionFailCount > 0 && failcount < testConnectionFailCount) { +failcount += 1 +println("failing in test") +throw new RuntimeException("failing...") + } + response.setStatusCode(testStatusCode); + if (testResponse != null) { +response.setEntity(new StringEntity(testResponse, StandardCharsets.UTF_8)) + } +} + }) +} + } + + mockServer.setUp() + val httpHost = mockServer.start() + val hostWithPort = s"${httpHost.getHostName}:${httpHost.getPort}" + + before { +testHang = 0.second +testStatusCode = 200 +testResponse = null +testConnectionFailCount = 0 +stream.reset() + } + + override def afterAll = { +mockServer.shutDown() + } + + behavior of "PoolingContainerClient" + + it should "not wait longer than set timeout" in { +val timeout = 5.seconds +val connection = new AkkaContainerClient(httpHost.getHostName, httpHost.getPort, timeout, 1.B, 100) +testHang = timeout * 2 +val start = Instant.now() +val result = Await.result(connection.post("/init", JsObject.empty, retry = true), 10.seconds) + +val end = Instant.now() +val waited = end.toEpochMilli - start.toEpochMilli +result shouldBe 'left +waited should be > timeout.toMillis +waited should be < (timeout * 2).toMillis + } + + it should "handle empty entity response" in { +val timeout = 5.seconds +val connection = new AkkaContainerClient(httpHost.getHostName, httpHost.getPort, timeout, 1.B, 100) +testStatusCode = 204 +val result = Await.result(connection.post("/init", JsObject.empty, retry = true), 10.seconds) +result shouldBe Left(NoResponseReceived()) + } + + it should "retry till timeout on StreamTcpException" in { +val
[GitHub] markusthoemmes commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils
markusthoemmes commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils URL: https://github.com/apache/incubator-openwhisk/pull/3812#discussion_r203990493 ## File path: tests/src/test/scala/whisk/core/containerpool/docker/test/AkkaContainerClientTests.scala ## @@ -0,0 +1,213 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package whisk.core.containerpool.docker.test + +import common.StreamLogging +import common.WskActorSystem +import java.nio.charset.StandardCharsets +import java.time.Instant +import org.apache.http.HttpRequest +import org.apache.http.HttpResponse +import org.apache.http.entity.StringEntity +import org.apache.http.localserver.LocalServerTestBase +import org.apache.http.protocol.HttpContext +import org.apache.http.protocol.HttpRequestHandler +import org.junit.runner.RunWith +import org.scalatest.BeforeAndAfter +import org.scalatest.BeforeAndAfterAll +import org.scalatest.FlatSpec +import org.scalatest.Matchers +import org.scalatest.junit.JUnitRunner +import scala.concurrent.Await +import scala.concurrent.TimeoutException +import scala.concurrent.duration._ +import spray.json.JsObject +import whisk.common.TransactionId +import whisk.core.containerpool.AkkaContainerClient +import whisk.core.entity.ActivationResponse._ +import whisk.core.entity.size._ + +/** + * Unit tests for AkkaContainerClientTests which communicate with containers. + */ +@RunWith(classOf[JUnitRunner]) +class AkkaContainerClientTests +extends FlatSpec +with Matchers +with BeforeAndAfter +with BeforeAndAfterAll +with StreamLogging +with WskActorSystem { + + implicit val transid = TransactionId.testing + implicit val ec = actorSystem.dispatcher + + var testHang: FiniteDuration = 0.second + var testStatusCode: Int = 200 + var testResponse: String = null + var testConnectionFailCount: Int = 0 + + val mockServer = new LocalServerTestBase { +var failcount = 0 +override def setUp() = { + super.setUp() + this.serverBootstrap +.registerHandler( + "/init", + new HttpRequestHandler() { +override def handle(request: HttpRequest, response: HttpResponse, context: HttpContext) = { + if (testHang.length > 0) { +Thread.sleep(testHang.toMillis) + } + if (testConnectionFailCount > 0 && failcount < testConnectionFailCount) { +failcount += 1 +println("failing in test") +throw new RuntimeException("failing...") + } + response.setStatusCode(testStatusCode); + if (testResponse != null) { +response.setEntity(new StringEntity(testResponse, StandardCharsets.UTF_8)) + } +} + }) +} + } + + mockServer.setUp() + val httpHost = mockServer.start() + val hostWithPort = s"${httpHost.getHostName}:${httpHost.getPort}" + + before { +testHang = 0.second +testStatusCode = 200 +testResponse = null +testConnectionFailCount = 0 +stream.reset() + } + + override def afterAll = { +mockServer.shutDown() + } + + behavior of "PoolingContainerClient" + + it should "not wait longer than set timeout" in { +val timeout = 5.seconds +val connection = new AkkaContainerClient(httpHost.getHostName, httpHost.getPort, timeout, 1.B, 100) +testHang = timeout * 2 +val start = Instant.now() +val result = Await.result(connection.post("/init", JsObject.empty, retry = true), 10.seconds) + +val end = Instant.now() +val waited = end.toEpochMilli - start.toEpochMilli +result shouldBe 'left +waited should be > timeout.toMillis +waited should be < (timeout * 2).toMillis + } + + it should "handle empty entity response" in { +val timeout = 5.seconds +val connection = new AkkaContainerClient(httpHost.getHostName, httpHost.getPort, timeout, 1.B, 100) +testStatusCode = 204 +val result = Await.result(connection.post("/init", JsObject.empty, retry = true), 10.seconds) +result shouldBe Left(NoResponseReceived()) + } + + it should "retry till timeout on StreamTcpException" in { +val
[GitHub] markusthoemmes commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils
markusthoemmes commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils URL: https://github.com/apache/incubator-openwhisk/pull/3812#discussion_r203974642 ## File path: common/scala/src/main/scala/whisk/core/containerpool/AkkaContainerClient.scala ## @@ -0,0 +1,216 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package whisk.core.containerpool + +import akka.actor.ActorSystem +import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport._ +import akka.http.scaladsl.marshalling.Marshal +import akka.http.scaladsl.model.HttpMethods +import akka.http.scaladsl.model.HttpRequest +import akka.http.scaladsl.model.HttpResponse +import akka.http.scaladsl.model.MediaTypes +import akka.http.scaladsl.model.MessageEntity +import akka.http.scaladsl.model.StatusCodes +import akka.http.scaladsl.model.headers.Accept +import akka.http.scaladsl.model.headers.Connection +import akka.http.scaladsl.unmarshalling.Unmarshal +import akka.stream.StreamTcpException +import akka.stream.scaladsl.Sink +import akka.stream.scaladsl.Source +import akka.util.ByteString +import scala.concurrent.Await +import scala.concurrent.ExecutionContext +import scala.concurrent.Future +import scala.concurrent.TimeoutException +import scala.concurrent.duration._ +import scala.util.Try +import scala.util.control.NonFatal +import spray.json._ +import whisk.common.Logging +import whisk.common.TransactionId +import whisk.core.entity.ActivationResponse.ContainerHttpError +import whisk.core.entity.ActivationResponse._ +import whisk.core.entity.ByteSize +import whisk.core.entity.size.SizeLong +import whisk.http.PoolingRestClient + +/** + * This HTTP client is used only in the invoker to communicate with the action container. + * It allows to POST a JSON object and receive JSON object back; that is the + * content type and the accept headers are both 'application/json. + * This implementation uses the akka http host-level client API. + * + * @param hostname the host name + * @param port the port + * @param timeout the timeout in msecs to wait for a response + * @param maxResponse the maximum size in bytes the connection will accept + * @param queueSize once all connections are used, how big of queue to allow for additional requests + * @param retryInterval duration between retries for TCP connection errors + */ +protected class AkkaContainerClient( + hostname: String, + port: Int, + timeout: FiniteDuration, + maxResponse: ByteSize, + queueSize: Int, + retryInterval: FiniteDuration = 100.milliseconds)(implicit logging: Logging, as: ActorSystem) +extends PoolingRestClient("http", hostname, port, queueSize, timeout = Some(timeout)) +with ContainerClient { + + def close() = Await.result(shutdown(), 30.seconds) + + /** + * Posts to hostname/endpoint the given JSON object. + * Waits up to timeout before aborting on a good connection. + * If the endpoint is not ready, retry up to timeout. + * Every retry reduces the available timeout so that this method should not + * wait longer than the total timeout (within a small slack allowance). + * + * @param endpoint the path the api call relative to hostname + * @param body the JSON value to post (this is usually a JSON objecT) + * @param retry whether or not to retry on connection failure + * @return Left(Error Message) or Right(Status Code, Response as UTF-8 String) + */ + def post(endpoint: String, body: JsValue, retry: Boolean)( +implicit tid: TransactionId): Future[Either[ContainerHttpError, ContainerResponse]] = { + +//create the request +val req = Marshal(body).to[MessageEntity].map { b => + //DO NOT reuse the connection + //For details on Connection: Close handling, see: + // - https://doc.akka.io/docs/akka-http/current/common/http-model.html#http-headers + // - http://github.com/akka/akka-http/tree/v10.1.3/akka-http-core/src/test/scala/akka/http/impl/engine/rendering/ResponseRendererSpec.scala#L470-L571 + HttpRequest(HttpMethods.POST, endpoint, entity = b) +.withHeaders(Connection("close"), Accept(MediaTypes.`application/json`)) +} + +retryingRequest(req, timeout, retry) +
[GitHub] markusthoemmes commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils
markusthoemmes commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils URL: https://github.com/apache/incubator-openwhisk/pull/3812#discussion_r203990965 ## File path: tests/src/test/scala/whisk/core/containerpool/docker/test/AkkaContainerClientTests.scala ## @@ -0,0 +1,213 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package whisk.core.containerpool.docker.test + +import common.StreamLogging +import common.WskActorSystem +import java.nio.charset.StandardCharsets +import java.time.Instant +import org.apache.http.HttpRequest +import org.apache.http.HttpResponse +import org.apache.http.entity.StringEntity +import org.apache.http.localserver.LocalServerTestBase +import org.apache.http.protocol.HttpContext +import org.apache.http.protocol.HttpRequestHandler +import org.junit.runner.RunWith +import org.scalatest.BeforeAndAfter +import org.scalatest.BeforeAndAfterAll +import org.scalatest.FlatSpec +import org.scalatest.Matchers +import org.scalatest.junit.JUnitRunner +import scala.concurrent.Await +import scala.concurrent.TimeoutException +import scala.concurrent.duration._ +import spray.json.JsObject +import whisk.common.TransactionId +import whisk.core.containerpool.AkkaContainerClient +import whisk.core.entity.ActivationResponse._ +import whisk.core.entity.size._ + +/** + * Unit tests for AkkaContainerClientTests which communicate with containers. + */ +@RunWith(classOf[JUnitRunner]) +class AkkaContainerClientTests +extends FlatSpec +with Matchers +with BeforeAndAfter +with BeforeAndAfterAll +with StreamLogging +with WskActorSystem { + + implicit val transid = TransactionId.testing + implicit val ec = actorSystem.dispatcher + + var testHang: FiniteDuration = 0.second + var testStatusCode: Int = 200 + var testResponse: String = null + var testConnectionFailCount: Int = 0 + + val mockServer = new LocalServerTestBase { +var failcount = 0 +override def setUp() = { + super.setUp() + this.serverBootstrap +.registerHandler( + "/init", + new HttpRequestHandler() { +override def handle(request: HttpRequest, response: HttpResponse, context: HttpContext) = { + if (testHang.length > 0) { +Thread.sleep(testHang.toMillis) + } + if (testConnectionFailCount > 0 && failcount < testConnectionFailCount) { +failcount += 1 +println("failing in test") +throw new RuntimeException("failing...") + } + response.setStatusCode(testStatusCode); + if (testResponse != null) { +response.setEntity(new StringEntity(testResponse, StandardCharsets.UTF_8)) + } +} + }) +} + } + + mockServer.setUp() + val httpHost = mockServer.start() + val hostWithPort = s"${httpHost.getHostName}:${httpHost.getPort}" + + before { +testHang = 0.second +testStatusCode = 200 +testResponse = null +testConnectionFailCount = 0 +stream.reset() + } + + override def afterAll = { +mockServer.shutDown() + } + + behavior of "PoolingContainerClient" + + it should "not wait longer than set timeout" in { +val timeout = 5.seconds +val connection = new AkkaContainerClient(httpHost.getHostName, httpHost.getPort, timeout, 1.B, 100) +testHang = timeout * 2 +val start = Instant.now() +val result = Await.result(connection.post("/init", JsObject.empty, retry = true), 10.seconds) + +val end = Instant.now() +val waited = end.toEpochMilli - start.toEpochMilli +result shouldBe 'left +waited should be > timeout.toMillis +waited should be < (timeout * 2).toMillis + } + + it should "handle empty entity response" in { +val timeout = 5.seconds +val connection = new AkkaContainerClient(httpHost.getHostName, httpHost.getPort, timeout, 1.B, 100) +testStatusCode = 204 +val result = Await.result(connection.post("/init", JsObject.empty, retry = true), 10.seconds) +result shouldBe Left(NoResponseReceived()) + } + + it should "retry till timeout on StreamTcpException" in { +val
[GitHub] markusthoemmes commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils
markusthoemmes commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils URL: https://github.com/apache/incubator-openwhisk/pull/3812#discussion_r203981358 ## File path: tests/src/test/scala/whisk/core/containerpool/test/ContainerPoolTests.scala ## @@ -296,7 +301,7 @@ class ContainerPoolTests val (containers, factory) = testContainers(2) val feed = TestProbe() -val pool = system.actorOf(ContainerPool.props(factory, ContainerPoolConfig(2, 2), feed.ref)) +val pool = system.actorOf(ContainerPool.props(factory, ContainerPoolConfig(2, 2, false), feed.ref)) Review comment: I think we should externalize building of the ContainerPoolConfig into a method so we don't need to adjust the values not needed for these tests continually. Check https://github.com/apache/incubator-openwhisk/pull/3767/files#diff-d00e1ef9ea3255332a28c35676361e29 for an impl. I did in another PR of exactly the same issue. I think it keeps the testcases clearer and reduces the diff in future PRs. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] markusthoemmes commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils
markusthoemmes commented on a change in pull request #3812: ContainerClient + akka http alternative to HttpUtils URL: https://github.com/apache/incubator-openwhisk/pull/3812#discussion_r203981897 ## File path: tests/src/test/scala/whisk/core/containerpool/kubernetes/test/KubernetesClientTests.scala ## @@ -188,6 +189,7 @@ object KubernetesClientTests { implicit def strToInstant(str: String): Instant = strToDate(str).get + implicit val as = ActorSystem("kubernetes-client-tests-actor-system") Review comment: This actorSystem is leaked I think. Could you make the `TestKubernetesClient` take the actorSystem as an implicit parameter, so you can use the one imported (and closed) by the tests above? Or maybe even move the class into the testclass. Makes it even easier? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ningyougang commented on issue #3619: Provide an activation store SPI
ningyougang commented on issue #3619: Provide an activation store SPI URL: https://github.com/apache/incubator-openwhisk/pull/3619#issuecomment-406547593 ### 1. Why need to write a new ActivationStoreProvider for `elasticsearch`? Currently, `activations` are saved into `couchdb` using `ArtifactActivationStore` couchdb has below disadvantage for saving activations * It is not convenience to `delete big activations`(activation data is growing very fast, when disk is full, need to delete, but should do compression at first, otherwise, disk usage will increase) * Couchdb's search feature is not stronger than `elasticsearch`. Since we provide `Provide an activation store SPI` in this patch, so i think it is necessary to implement a new ActivationStoreProvider for `elasticsearch` ### 2.How to implement it? * Implement this trait `ActivationStore`, for example: ``` class ElasticsearchActivationStore(actorSystem: ActorSystem, actorMaterializer: ActorMaterializer, logging: Logging) extends ActivationStore ``` * In class ElasticsearchActivationStore inner, we can use `https://github.com/sksamuel/elastic4s/` to implement it. @markusthoemmes ,how do you think? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] markusthoemmes commented on issue #3619: Provide an activation store SPI
markusthoemmes commented on issue #3619: Provide an activation store SPI URL: https://github.com/apache/incubator-openwhisk/pull/3619#issuecomment-406529803 @dubee can give you more information on plans here. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services