[GitHub] dubeejw closed pull request #2866: only build invoker and controller
dubeejw closed pull request #2866: only build invoker and controller URL: https://github.com/apache/incubator-openwhisk/pull/2866 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/tools/jenkins/apache/dockerhub.groovy b/tools/jenkins/apache/dockerhub.groovy index 975e1961c4..c804b23d34 100644 --- a/tools/jenkins/apache/dockerhub.groovy +++ b/tools/jenkins/apache/dockerhub.groovy @@ -12,7 +12,7 @@ node('xenial&&!H21&&!H22&&!H11&&!ubuntu-eu3') { withCredentials([usernamePassword(credentialsId: 'openwhisk_dockerhub', passwordVariable: 'DOCKER_PASSWORD', usernameVariable: 'DOCKER_USER')]) { sh 'docker login -u ${DOCKER_USER} -p ${DOCKER_PASSWORD}' } - def PUSH_CMD = "./gradlew distDocker -PdockerRegistry=docker.io -PdockerImagePrefix=openwhisk -x tests:dat:blackbox:badproxy:distDocker -x tests:dat:blackbox:badaction:distDocker -x sdk:docker:distDocker -x tools:cli:distDocker -x tools:cli:distDocker" + def PUSH_CMD = "./gradlew :core:controller:distDocker :core:invoker:distDocker -PdockerRegistry=docker.io -PdockerImagePrefix=openwhisk" def gitCommit = sh(returnStdout: true, script: 'git rev-parse HEAD').trim() def shortCommit = gitCommit.take(7) sh "${PUSH_CMD} -PdockerImageTag=latest" 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] dubeejw closed issue #2865: only deploy invoker and controller with jenkins
dubeejw closed issue #2865: only deploy invoker and controller with jenkins URL: https://github.com/apache/incubator-openwhisk/issues/2865 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 #2850: Move docker runtime into it's own repo
rabbah closed pull request #2850: Move docker runtime into it's own repo URL: https://github.com/apache/incubator-openwhisk/pull/2850 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/ansible/edge.yml b/ansible/edge.yml index cf80ebeb2d..6566d572ae 100644 --- a/ansible/edge.yml +++ b/ansible/edge.yml @@ -8,4 +8,3 @@ roles: - nginx - cli - - sdk diff --git a/ansible/roles/nginx/templates/nginx.conf.j2 b/ansible/roles/nginx/templates/nginx.conf.j2 index 31c1ccfe08..6a0b4dc875 100644 --- a/ansible/roles/nginx/templates/nginx.conf.j2 +++ b/ansible/roles/nginx/templates/nginx.conf.j2 @@ -76,8 +76,12 @@ http { proxy_read_timeout 70s; # 60+10 additional seconds to allow controller to terminate request } +location /blackbox.tar.gz { +return 301 https://github.com/apache/incubator-openwhisk-runtime-docker/releases/download/sdk%400.1.0/blackbox-0.1.0.tar.gz; +} +# leaving this for a while for clients out there to update to the new endpoint location /blackbox-0.1.0.tar.gz { -root /etc/nginx; +return 301 /blackbox.tar.gz; } location /OpenWhiskIOSStarterApp.zip { diff --git a/ansible/roles/sdk/tasks/clean.yml b/ansible/roles/sdk/tasks/clean.yml deleted file mode 100644 index c9e83edeca..00 --- a/ansible/roles/sdk/tasks/clean.yml +++ /dev/null @@ -1,8 +0,0 @@ -# Remove SDK artifacts from nginx. - -- name: remove blackbox sdk - file: -path: "{{ nginx.confdir }}/blackbox-0.1.0.tar.gz" -state: absent - become: "{{ sdk.dir.become }}" diff --git a/ansible/roles/sdk/tasks/deploy.yml b/ansible/roles/sdk/tasks/deploy.yml deleted file mode 100644 index 4b6b2d..00 --- a/ansible/roles/sdk/tasks/deploy.yml +++ /dev/null @@ -1,39 +0,0 @@ -# Tasks for handling SDK generation and publishing - -- name: ensure nginx config directory exists - file: -path: "{{ nginx.confdir }}" -state: directory - become: "{{ sdk.dir.become }}" - -# Blackbox - -- name: make temp dir - local_action: shell "mktemp" "-d" "{{ lookup('env', 'TMPDIR') | default('/tmp', true) }}/whisk." - register: tmpdir - -- name: copy docker sdk to dockerSkeleton in scratch space - local_action: copy src="{{ openwhisk_home }}/sdk/docker/{{ item }}" dest="{{ tmpdir.stdout }}/dockerSkeleton/" - with_items: -- buildAndPush.sh -- Dockerfile -- example.c -- README.md - -- name: rename base image in Dockerfile - local_action: replace dest="{{ tmpdir.stdout }}/dockerSkeleton/Dockerfile" regexp='^FROM dockerskeleton*.*$' replace='FROM openwhisk/dockerskeleton' - -- name: fix file permissions - local_action: file path="{{ tmpdir.stdout }}/dockerSkeleton/buildAndPush.sh" mode=0755 - -- name: build blackbox container artifact - local_action: shell "tar" "--exclude=build.gradle" "-czf" "{{ tmpdir.stdout }}/blackbox-0.1.0.tar.gz" "dockerSkeleton" chdir="{{ tmpdir.stdout }}" - -- name: copy blackbox container artifact to nginx - copy: -src: "{{ tmpdir.stdout }}/blackbox-0.1.0.tar.gz" -dest: "{{ nginx.confdir }}" - -- name: remove temp dir - local_action: file path="{{ tmpdir.stdout }}" state=absent diff --git a/ansible/roles/sdk/tasks/main.yml b/ansible/roles/sdk/tasks/main.yml deleted file mode 100644 index 32f892d54e..00 --- a/ansible/roles/sdk/tasks/main.yml +++ /dev/null @@ -1,10 +0,0 @@ -# This role will build and publish Openwhisk SDKs. Currently covers blackbox starter kit. -# In deploy mode it will generate downloadable artifacts and copy them to nginx. -# In clean mode it will remove the artifacts from nginx. - -- include: deploy.yml - when: mode == "deploy" - -- include: clean.yml - when: mode == "clean" diff --git a/core/actionProxy/Dockerfile b/core/actionProxy/Dockerfile index 6ec4ff4f38..16094a719b 100644 --- a/core/actionProxy/Dockerfile +++ b/core/actionProxy/Dockerfile @@ -1,23 +1,2 @@ # Dockerfile for docker skeleton (useful for running blackbox binaries, scripts, or Python 3 actions) . -FROM python:3.6.1-alpine - -# Upgrade and install basic Python dependencies. -RUN apk add --no-cache bash \ - && apk add --no-cache --virtual .build-deps \ -bzip2-dev \ -gcc \ -libc-dev \ - && pip install --upgrade pip setuptools six \ - && pip install --no-cache-dir gevent==1.2.1 flask==0.12 \ - && apk del .build-deps - -ENV FLASK_PROXY_PORT 8080 - -RUN mkdir -p /actionProxy
[GitHub] dubeejw closed pull request #217: Add ability to get trigger configuration and status
dubeejw closed pull request #217: Add ability to get trigger configuration and status URL: https://github.com/apache/incubator-openwhisk-package-kafka/pull/217 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/action/kafkaFeed.js b/action/kafkaFeed.js index 3cd42f7..bf55538 100644 --- a/action/kafkaFeed.js +++ b/action/kafkaFeed.js @@ -18,6 +18,8 @@ function main(params) { if (params.lifecycleEvent === 'CREATE') { return common.createTrigger(endpoint, massagedParams, webActionName); +} else if (params.lifecycleEvent === 'READ') { +return common.getTrigger(endpoint, massagedParams, webActionName); } else if (params.lifecycleEvent === 'DELETE') { return common.deleteTrigger(endpoint, massagedParams, webActionName); } diff --git a/action/kafkaFeedWeb.js b/action/kafkaFeedWeb.js index de39edb..d60ce2f 100644 --- a/action/kafkaFeedWeb.js +++ b/action/kafkaFeedWeb.js @@ -61,6 +61,35 @@ function main(params) { resolve(common.webResponse(statusCode, body)); }); +} else if (params.__ow_method === "get") { +const triggerURL = common.getTriggerURL(params.authKey, params.endpoint, params.triggerName); + +return common.verifyTriggerAuth(triggerURL) +.then(() => { +db = new Database(params.DB_URL, params.DB_NAME); +return db.getTrigger(params.triggerName); +}) +.then((triggerDoc) => { +var body = { +config: { +triggerName: triggerDoc.triggerName, +topic: triggerDoc.topic, +isJSONData: triggerDoc.isJSONData, +isBinaryValue: triggerDoc.isBinaryValue, +isBinaryKey: triggerDoc.isBinaryKey, +isMessageHub: triggerDoc.isMessageHub, +brokers: triggerDoc.brokers, +kafka_admin_url: triggerDoc.kafka_admin_url, +username: triggerDoc.username, +password: triggerDoc.password +}, +status: triggerDoc.status +} +resolve(common.webResponse(200, body, 'application/json')); +}) +.catch(error => { +resolve(common.webResponse(500, error.toString())); +}); } else if (params.__ow_method === "delete") { const triggerURL = common.getTriggerURL(params.authKey, params.endpoint, params.triggerName); diff --git a/action/lib/common.js b/action/lib/common.js index 46a7d32..638f8c2 100644 --- a/action/lib/common.js +++ b/action/lib/common.js @@ -130,6 +130,30 @@ function deleteTrigger(endpoint, params, actionName) { }); } +function getTrigger(endpoint, params, actionName) { +var options = { +method: 'GET', +url: getWebActionURL(endpoint, actionName), +rejectUnauthorized: false, +json: true, +body: params, +headers: { +'Content-Type': 'application/json', +'Accept': 'application/json', +'User-Agent': 'whisk' +} +}; + +return request(options) +.then(response => { +return response; +}) +.catch(error => { +console.log(`Error fetching trigger: ${JSON.stringify(error, null, 2)}`); +return Promise.reject(error.response.body); +}); +} + // perform parameter validation that is common to both feed actions function performCommonParameterValidation(rawParams) { var validatedParams = {}; @@ -166,11 +190,11 @@ function performCommonParameterValidation(rawParams) { return { validatedParams: validatedParams }; } -function webResponse(code, body) { +function webResponse(code, body, contentType = 'text/plain') { return { statusCode: code, headers: { -'Content-Type': 'text/plain' +'Content-Type': contentType }, body: body }; @@ -179,6 +203,7 @@ function webResponse(code, body) { module.exports = { 'createTrigger': createTrigger, 'deleteTrigger': deleteTrigger, +'getTrigger': getTrigger, 'getBooleanFromArgs': getBooleanFromArgs, 'getTriggerFQN': getTriggerFQN, 'getTriggerURL': getTrigg
[GitHub] csantanapr commented on a change in pull request #2853: update kinds
csantanapr commented on a change in pull request #2853: update kinds URL: https://github.com/apache/incubator-openwhisk/pull/2853#discussion_r145161553 ## File path: core/controller/src/main/resources/apiv1swagger.json ## @@ -1554,13 +1554,10 @@ "kind": { "type": "string", "enum": [ -"nodejs", "nodejs:6", -"nodejs:default", -"python", -"swift", -"swift:3", -"swift:default", +"python:2", +"python:3", +"swift:3.1.1", Review comment: it's encouragement documentation for folks to use `swift:3.1.1` :-) I can add back, and remove later when we deprecate and announce in mailing list 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] dubeejw commented on a change in pull request #217: Add ability to get trigger configuration and status
dubeejw commented on a change in pull request #217: Add ability to get trigger configuration and status URL: https://github.com/apache/incubator-openwhisk-package-kafka/pull/217#discussion_r145153254 ## File path: tests/src/test/scala/system/packages/MessageHubFeedTests.scala ## @@ -274,6 +274,67 @@ class MessageHubFeedTests assert(matchingActivations.length == 0) } + it should "return correct status and configuration" in withAssetCleaner(wskprops) { +val currentTime = s"${System.currentTimeMillis}" + +(wp, assetHelper) => + val triggerName = s"/_/dummyMessageHubTrigger-$currentTime" + println(s"Creating trigger ${triggerName}") + + val username = kafkaUtils.getAsJson("user") + val password = kafkaUtils.getAsJson("password") + val admin_url = kafkaUtils.getAsJson("kafka_admin_url") + val brokers = kafkaUtils.getAsJson("brokers") + + createTrigger(assetHelper, triggerName, parameters = Map( +"user" -> username, +"password" -> password, +"api_key" -> kafkaUtils.getAsJson("api_key"), +"kafka_admin_url" -> admin_url, +"kafka_brokers_sasl" -> brokers, +"topic" -> topic.toJson, +"isBinaryKey" -> false.toJson, +"isBinaryValue" -> false.toJson + )) + + val run = wsk.action.invoke(actionName, parameters = Map( Review comment: I understand that the trigger will not be fetched on the read based on how the OpenWhisk infrastructure is setup. However, is there a test to make sure this provider errors as expected? 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] dubeejw commented on a change in pull request #217: Add ability to get trigger configuration and status
dubeejw commented on a change in pull request #217: Add ability to get trigger configuration and status URL: https://github.com/apache/incubator-openwhisk-package-kafka/pull/217#discussion_r145153451 ## File path: action/kafkaFeedWeb.js ## @@ -61,6 +61,35 @@ function main(params) { resolve(common.webResponse(statusCode, body)); }); +} else if (params.__ow_method === "get") { Review comment: Can you open an issue for this? 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] dubeejw commented on a change in pull request #217: Add ability to get trigger configuration and status
dubeejw commented on a change in pull request #217: Add ability to get trigger configuration and status URL: https://github.com/apache/incubator-openwhisk-package-kafka/pull/217#discussion_r145149823 ## File path: tests/src/test/scala/system/packages/MessageHubFeedTests.scala ## @@ -274,6 +274,67 @@ class MessageHubFeedTests assert(matchingActivations.length == 0) } + it should "return correct status and configuration" in withAssetCleaner(wskprops) { +val currentTime = s"${System.currentTimeMillis}" + +(wp, assetHelper) => + val triggerName = s"/_/dummyMessageHubTrigger-$currentTime" + println(s"Creating trigger ${triggerName}") + + val username = kafkaUtils.getAsJson("user") + val password = kafkaUtils.getAsJson("password") + val admin_url = kafkaUtils.getAsJson("kafka_admin_url") + val brokers = kafkaUtils.getAsJson("brokers") + + createTrigger(assetHelper, triggerName, parameters = Map( +"user" -> username, +"password" -> password, +"api_key" -> kafkaUtils.getAsJson("api_key"), +"kafka_admin_url" -> admin_url, +"kafka_brokers_sasl" -> brokers, +"topic" -> topic.toJson, +"isBinaryKey" -> false.toJson, +"isBinaryValue" -> false.toJson + )) + + val run = wsk.action.invoke(actionName, parameters = Map( +"triggerName" -> triggerName.toJson, +"lifecycleEvent" -> "READ".toJson, +"authKey" -> wp.authKey.toJson + )) + + withActivation(wsk.activation, run) { +activation => + activation.response.success shouldBe true + + inside (activation.response.result) { +case Some(result) => + val config = result.getFields("config").head.asInstanceOf[JsObject].fields + val status = result.getFields("status").head.asInstanceOf[JsObject].fields + + config should contain("brokers" -> brokers) Review comment: I see two different possible values for dataChanged. ![screen shot 2017-10-17 at 10 33 53 am](https://user-images.githubusercontent.com/11520887/31670932-c4504d26-b326-11e7-82fe-e1b7de6dce20.png) ---- 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] dubeejw commented on a change in pull request #217: Add ability to get trigger configuration and status
dubeejw commented on a change in pull request #217: Add ability to get trigger configuration and status URL: https://github.com/apache/incubator-openwhisk-package-kafka/pull/217#discussion_r145149185 ## File path: tests/src/test/scala/system/packages/MessageHubFeedTests.scala ## @@ -274,6 +274,67 @@ class MessageHubFeedTests assert(matchingActivations.length == 0) } + it should "return correct status and configuration" in withAssetCleaner(wskprops) { +val currentTime = s"${System.currentTimeMillis}" + +(wp, assetHelper) => + val triggerName = s"/_/dummyMessageHubTrigger-$currentTime" + println(s"Creating trigger ${triggerName}") + + val username = kafkaUtils.getAsJson("user") + val password = kafkaUtils.getAsJson("password") + val admin_url = kafkaUtils.getAsJson("kafka_admin_url") + val brokers = kafkaUtils.getAsJson("brokers") + + createTrigger(assetHelper, triggerName, parameters = Map( +"user" -> username, +"password" -> password, +"api_key" -> kafkaUtils.getAsJson("api_key"), +"kafka_admin_url" -> admin_url, +"kafka_brokers_sasl" -> brokers, +"topic" -> topic.toJson, +"isBinaryKey" -> false.toJson, +"isBinaryValue" -> false.toJson + )) + + val run = wsk.action.invoke(actionName, parameters = Map( +"triggerName" -> triggerName.toJson, +"lifecycleEvent" -> "READ".toJson, +"authKey" -> wp.authKey.toJson + )) + + withActivation(wsk.activation, run) { +activation => + activation.response.success shouldBe true + + inside (activation.response.result) { +case Some(result) => + val config = result.getFields("config").head.asInstanceOf[JsObject].fields + val status = result.getFields("status").head.asInstanceOf[JsObject].fields + + config should contain("brokers" -> brokers) Review comment: So we know what the returned trigger name will be. That means the config property can be compared to a JsObject. What does dateChanged look like? Is that a timestamp? ---- 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 closed pull request #2648: Overhaul logging
markusthoemmes closed pull request #2648: Overhaul logging URL: https://github.com/apache/incubator-openwhisk/pull/2648 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 #2648: Overhaul logging
markusthoemmes commented on issue #2648: Overhaul logging URL: https://github.com/apache/incubator-openwhisk/pull/2648#issuecomment-337253953 @mhenke1 is doing all this 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] rabbah closed pull request #2812: Implement bulk write of activations in the Invoker.
rabbah closed pull request #2812: Implement bulk write of activations in the Invoker. URL: https://github.com/apache/incubator-openwhisk/pull/2812 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 closed pull request #2689: first stage of support for dynamic invoker id assignment
markusthoemmes closed pull request #2689: first stage of support for dynamic invoker id assignment URL: https://github.com/apache/incubator-openwhisk/pull/2689 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 #2729: add support for multiple versions of ubuntu
markusthoemmes commented on a change in pull request #2729: add support for multiple versions of ubuntu URL: https://github.com/apache/incubator-openwhisk/pull/2729#discussion_r145144663 ## File path: tools/ubuntu-setup/docker.sh ## @@ -14,7 +14,7 @@ sudo apt-cache policy docker-engine sudo apt-get -y install linux-image-extra-$(uname -r) # DOCKER -sudo apt-get install -y --force-yes docker-engine=1.12.0-0~trusty +sudo apt-get install -y --force-yes docker-engine Review comment: This is a bit dangerous. While OpenWhisk works on newer docker versions perfectly fine, it is not CIed as of today. We got bitten by docker version regressions so we very carefully update and today recommend using 1.12 for anything close to a production environment. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rabbah commented on issue #2850: Move docker runtime into it's own repo
rabbah commented on issue #2850: Move docker runtime into it's own repo URL: https://github.com/apache/incubator-openwhisk/pull/2850#issuecomment-337242055 can you rebase this? 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] dubeejw commented on issue #2544: Allow CLI to Save Code from Action
dubeejw commented on issue #2544: Allow CLI to Save Code from Action URL: https://github.com/apache/incubator-openwhisk/pull/2544#issuecomment-337241809 @mdeuser, made changes as requested, thanks. Anything else? 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] dubeejw commented on issue #2544: Allow CLI to Save Code from Action
dubeejw commented on issue #2544: Allow CLI to Save Code from Action URL: https://github.com/apache/incubator-openwhisk/pull/2544#issuecomment-337241702 PG4 895 ? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rabbah commented on a change in pull request #2853: update kinds
rabbah commented on a change in pull request #2853: update kinds URL: https://github.com/apache/incubator-openwhisk/pull/2853#discussion_r145139789 ## File path: core/controller/src/main/resources/apiv1swagger.json ## @@ -1554,13 +1554,10 @@ "kind": { "type": "string", "enum": [ -"nodejs", "nodejs:6", -"nodejs:default", -"python", -"swift", -"swift:3", -"swift:default", +"python:2", +"python:3", +"swift:3.1.1", Review comment: dropped swift 3. 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 #2843: Move python runtime into it's own repo
rabbah closed pull request #2843: Move python runtime into it's own repo URL: https://github.com/apache/incubator-openwhisk/pull/2843 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 #2823: Move php runtime into it's own repo
rabbah closed pull request #2823: Move php runtime into it's own repo URL: https://github.com/apache/incubator-openwhisk/pull/2823 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 #2822: Move swift runtime into it's own repo
rabbah closed pull request #2822: Move swift runtime into it's own repo URL: https://github.com/apache/incubator-openwhisk/pull/2822 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rabbah commented on a change in pull request #2822: Move swift runtime into it's own repo
rabbah commented on a change in pull request #2822: Move swift runtime into it's own repo URL: https://github.com/apache/incubator-openwhisk/pull/2822#discussion_r145138909 ## File path: core/swift3Action/Dockerfile ## @@ -1,9 +1 @@ -# Dockerfile for swift actions, overrides and extends ActionRunner from actionProxy -# This Dockerfile is partially based on: https://github.com/swiftdocker/docker-swift/ - -# Temporarily disable Swift3 action container image build because it causes a build break. -# It has a dependency on https://github.com/IBM-Swift/swift-watson-sdk.git -# which is no more available. -# Just base on a fixed version which is available on public DockerHub - -FROM openwhisk/swift3action:03023a1 \ No newline at end of file +FROM openwhisk/swift3action:1.0.0 Review comment: could have adopted uniform naming now at least from the canonical repos. 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 #2821: Move java runtime into it's own repo
rabbah closed pull request #2821: Move java runtime into it's own repo URL: https://github.com/apache/incubator-openwhisk/pull/2821 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rabbah commented on issue #2821: Move java runtime into it's own repo
rabbah commented on issue #2821: Move java runtime into it's own repo URL: https://github.com/apache/incubator-openwhisk/pull/2821#issuecomment-337239418 @csantanapr i think it would make sense after all these prs are merged to have a runtimes top level directory (peer with core) and put all the dockerfiles there. thoughts? 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 #2834: Move nodejs runtime into it's own repo
rabbah closed pull request #2834: Move nodejs runtime into it's own repo URL: https://github.com/apache/incubator-openwhisk/pull/2834 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] dgrove-oss commented on issue #2689: first stage of support for dynamic invoker id assignment
dgrove-oss commented on issue #2689: first stage of support for dynamic invoker id assignment URL: https://github.com/apache/incubator-openwhisk/pull/2689#issuecomment-337231124 rebased yet again to master to resolve conflicts. PG1 2150 ? Could we please finally merge once CI approves? 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] mhenke1 commented on a change in pull request #2857: Emit metrics via kamon - wip
mhenke1 commented on a change in pull request #2857: Emit metrics via kamon - wip URL: https://github.com/apache/incubator-openwhisk/pull/2857#discussion_r145129065 ## File path: common/scala/src/main/scala/whisk/common/Logging.scala ## @@ -178,6 +179,34 @@ object LogMarkerToken { } } +trait WhiskMetric {} + +case class WhiskCounterMetric(component: String, action: String) extends WhiskMetric {} + +case class WhiskHistogramMetric(component: String, action: String, value: Long) extends WhiskMetric + +object MetricEmitter { + + def getMetricName(component: String, action: String, isFailed: Boolean = false): String = { +if (isFailed) { + component + "_" + action + "_failed" +} else { + component + "_" + action +} + } + + def emitMetric(wm: WhiskMetric, isFailed: Boolean = false) = wm match { +case WhiskCounterMetric(component, action) => + Kamon.metrics +.counter(getMetricName(component, action, isFailed)) +.increment(1) +case WhiskHistogramMetric(component, action, value) => + Kamon.metrics +.histogram(getMetricName(component, action, isFailed)) +.record(value) + } Review comment: I have simplified the MetricsEmitter (via amend) to reduce allocations 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] mhenke1 commented on a change in pull request #2857: Emit metrics via kamon - wip
mhenke1 commented on a change in pull request #2857: Emit metrics via kamon - wip URL: https://github.com/apache/incubator-openwhisk/pull/2857#discussion_r145128800 ## File path: common/scala/src/main/scala/whisk/common/TransactionId.scala ## @@ -55,7 +55,14 @@ case class TransactionId private (meta: TransactionMetadata) extends AnyVal { */ def mark(from: AnyRef, marker: LogMarkerToken, message: String = "", logLevel: LogLevel = InfoLevel)( implicit logging: Logging) = { -logging.emit(logLevel, this, from, createMessageWithMarker(message, LogMarker(marker, deltaToStart))) + +if (meta.metricsLog) { + logging.emit(logLevel, this, from, createMessageWithMarker(message, LogMarker(marker, deltaToStart))) +} Review comment: I changed the PR (via amend) to emit simple log message in the original debug level to avoid loss of information. I will tackle log reduction in a following commit 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] strajansebastian commented on issue #2729: add support for multiple versions of ubuntu
strajansebastian commented on issue #2729: add support for multiple versions of ubuntu URL: https://github.com/apache/incubator-openwhisk/pull/2729#issuecomment-337230432 any update on this? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rabbah commented on issue #74: Adding support for responsetype parameter during route create.
rabbah commented on issue #74: Adding support for responsetype parameter during route create. URL: https://github.com/apache/incubator-openwhisk-client-js/pull/74#issuecomment-337193736 I?ll add the release note. 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 #74: Adding support for responsetype parameter during route create.
rabbah closed pull request #74: Adding support for responsetype parameter during route create. URL: https://github.com/apache/incubator-openwhisk-client-js/pull/74 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 #2665: Add ability to use all controllers round robin
markusthoemmes commented on a change in pull request #2665: Add ability to use all controllers round robin URL: https://github.com/apache/incubator-openwhisk/pull/2665#discussion_r145068749 ## File path: tests/src/test/scala/whisk/core/database/test/CacheConcurrencyTests.scala ## @@ -121,14 +124,24 @@ class CacheConcurrencyTests extends FlatSpec with WskTestHelpers with BeforeAndA } } - run("get after update") { name => -wsk.action.get(name) - } map { -case (name, rr) => - withClue(s"get after update: failed check for $name") { -rr.stdout should include("blue") -rr.stdout should not include ("red") + // All controllers should have the correct action + // As they are used round robin, we ask every controller for the action. + // We add a retry to tollarate a short interval to bring the controllers in sync. + retry( +{ + (1 to WhiskProperties.getControllerHosts.split(",").length).foreach { _ => Review comment: aka ```scala WhiskProperties.getControllerHosts.split(",").foreach { _ => ``` ? -------- 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] jthomas commented on issue #74: Adding support for responsetype parameter during route create.
jthomas commented on issue #74: Adding support for responsetype parameter during route create. URL: https://github.com/apache/incubator-openwhisk-client-js/pull/74#issuecomment-337161562 @csantanapr @rabbah ^^ 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 closed pull request #2828: option to use docker pause/unpause instead of docker-runc pause/unpause
markusthoemmes closed pull request #2828: option to use docker pause/unpause instead of docker-runc pause/unpause URL: https://github.com/apache/incubator-openwhisk/pull/2828 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] sven-lange-last commented on issue #2858: Make Docker action container cleanup during invoker startup / shutdown more robust
sven-lange-last commented on issue #2858: Make Docker action container cleanup during invoker startup / shutdown more robust URL: https://github.com/apache/incubator-openwhisk/pull/2858#issuecomment-336915644 PG 1 / 2143 successful. 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] hykych closed issue #2856: How do you limit the concurrent activations with multiple Controller?
hykych closed issue #2856: How do you limit the concurrent activations with multiple Controller? URL: https://github.com/apache/incubator-openwhisk/issues/2856 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] hykych opened a new issue #2856: How do you limit the concurrent activations with multiple Controller?
hykych opened a new issue #2856: How do you limit the concurrent activations with multiple Controller? URL: https://github.com/apache/incubator-openwhisk/issues/2856 I noticed that you use the `activationByNamespaceId` in `LoadBalancerData` to count the concurrent activations. However, this is invalid with multiple Controller. Is there anything I didn't noticed or I made a mistake on that code snippet? 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] hykych closed issue #2733: Have seen your Invoker inplementation, how's its performence and memory cost?
hykych closed issue #2733: Have seen your Invoker inplementation, how's its performence and memory cost? URL: https://github.com/apache/incubator-openwhisk/issues/2733 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] hykych commented on issue #2733: Have seen your Invoker inplementation, how's its performence and memory cost?
hykych commented on issue #2733: Have seen your Invoker inplementation, how's its performence and memory cost? URL: https://github.com/apache/incubator-openwhisk/issues/2733#issuecomment-337089298 @markusthoemmes @rabbah thanks This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] hykych commented on issue #2856: How do you limit the concurrent activations with multiple Controller?
hykych commented on issue #2856: How do you limit the concurrent activations with multiple Controller? URL: https://github.com/apache/incubator-openwhisk/issues/2856#issuecomment-337089203 @vvraskin thanks This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] hykych closed issue #2856: How do you limit the concurrent activations with multiple Controller?
hykych closed issue #2856: How do you limit the concurrent activations with multiple Controller? URL: https://github.com/apache/incubator-openwhisk/issues/2856 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 #2859: LoadBalancerService does not consider concurrent requests when scheduling
markusthoemmes commented on issue #2859: LoadBalancerService does not consider concurrent requests when scheduling URL: https://github.com/apache/incubator-openwhisk/issues/2859#issuecomment-337065359 This is not necessarily related to the shared state, it is also the case with the local version of the load balancer. Although that used AtomicIntegers, they were simply used as a thread-safe counter in terms of changing operations, but not used as an atomically checked-then-update counter. Should we implement the Loadbalancer as an Actor to achieve local consistency for each Controller to start? That would solve parallel read consistency at least. For the write/read races introduced because of state-sharing, we can come up with a smart solution as outlined by @tysonnorris to keep local state synchronous. Maybe there's a mechanism to wait for a write to happen and then implement the respective operations as `ask`s so they "block" scheduling until that returned? As noted by @vvraskin both of these bits would need to be tested under load but I'm confident that at least the "load balancer as an Actor" should not impose a bottleneck. 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 #2859: LoadBalancerService does not consider concurrent requests when scheduling
tysonnorris commented on issue #2859: LoadBalancerService does not consider concurrent requests when scheduling URL: https://github.com/apache/incubator-openwhisk/issues/2859#issuecomment-337057445 I think "reading your own writes" is only for the replicated data, which is inherently async (by way of messaging with replicator), so I'm not sure that will work (but great if it does!). I was thinking about the following: - revert to the LoadBalancerData to sync signatures (e.g. `def activationCountPerInvoker: Map[String, Int]` - make DistributedLoadBalancerData keep 2 sets of data - one local, one replicated - the views that DistributedLoadBalancerData exposes would be a composition of local + replicated data Note that this requires changing the replicated format to include an indication of the controller instance that contributed the data, so that the local instance can be ignored within the set of replicated data, in favor of the more accurate local data. Basically the localdata would be accurate and the replicated data would be eventually consistent. But at least within the same controller the scheduling would be predictable under load. 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] dubeejw commented on a change in pull request #2544: Allow CLI to Save Code from Action
dubeejw commented on a change in pull request #2544: Allow CLI to Save Code from Action URL: https://github.com/apache/incubator-openwhisk/pull/2544#discussion_r144978426 ## File path: tests/src/test/scala/whisk/core/cli/test/WskBasicUsageTests.scala ## @@ -811,6 +813,81 @@ class WskBasicUsageTests extends TestHelpers with WskTestHelpers { stdoutNoDescOrParams should include regex (s"(?i)action /${namespace}/${actNameNoDescOrParams}\\s*\\(parameters: none defined\\)") } + it should "save action code to file" in withAssetCleaner(wskprops) { (wp, assetHelper) => +val name = "saveAction" +val seqName = "seqName" +val dockerName = "dockerName" +val containerName = s"bogus${Random.alphanumeric.take(16).mkString.toLowerCase}" +val saveName = s"save-as-$name.js" +val badSaveName = s"bad-directory${File.separator}$saveName" + +// Test for successful --save +assetHelper.withCleaner(wsk.action, name) { (action, _) => + action.create(name, defaultAction) +} + +val saveMsg = wsk.action.get(name, save = Some(true)).stdout + +saveMsg should include(s"saved action code to ") + +val savePath = saveMsg.split("ok: saved action code to ")(1).trim() +val saveFile = new File(savePath); + +try { + saveFile.exists shouldBe true + + // Test for failure saving file when it already exist + wsk.action.get(name, save = Some(true), expectedExitCode = MISUSE_EXIT).stderr should include( +s"The file '$name.js' already exists") +} finally { + saveFile.delete() +} + +// Test for successful --save-as +val saveAsMsg = wsk.action.get(name, saveAs = Some(saveName)).stdout + +saveAsMsg should include(s"saved action code to ") + +val saveAsPath = saveAsMsg.split("ok: saved action code to ")(1).trim() +val saveAsFile = new File(saveAsPath); + +try { + saveAsFile.exists shouldBe true + + // Test for failure saving file when it already exist + wsk.action.get(name, saveAs = Some(saveName), expectedExitCode = MISUSE_EXIT).stderr should include( +s"The file '$saveName' already exists") +} finally { + saveAsFile.delete() +} + +// Test for failure when using an invalid filename +wsk.action.get(name, saveAs = Some(badSaveName), expectedExitCode = MISUSE_EXIT).stderr should include( + s"Cannot create file '$badSaveName'") + +// Test for failure saving Docker images +assetHelper.withCleaner(wsk.action, dockerName) { (action, _) => + action.create(dockerName, None, docker = Some(containerName)) +} + +wsk.action.get(dockerName, save = Some(true), expectedExitCode = MISUSE_EXIT).stderr should include( + "Cannot save Docker images") + +wsk.action.get(dockerName, saveAs = Some(dockerName), expectedExitCode = MISUSE_EXIT).stderr should include( + "Cannot save Docker images") + +// Tes for failure saving sequences +assetHelper.withCleaner(wsk.action, seqName) { (action, _) => + action.create(seqName, Some(name), kind = Some("sequence")) +} + +wsk.action.get(seqName, save = Some(true), expectedExitCode = MISUSE_EXIT).stderr should include( + "Cannot save action sequence") Review comment: Yes. 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 commented on a change in pull request #2544: Allow CLI to Save Code from Action
mdeuser commented on a change in pull request #2544: Allow CLI to Save Code from Action URL: https://github.com/apache/incubator-openwhisk/pull/2544#discussion_r144964356 ## File path: tools/cli/go-whisk-cli/commands/action.go ## @@ -495,6 +513,87 @@ func getExec(args []string, params ActionFlags) (*whisk.Exec, error) { return exec, nil } +func getBinaryKindExtension(kind string) (extension string){ +switch strings.ToLower(kind) { +case JAVA: +extension = JAVA_EXT +default: +extension = ZIP_EXT +} + +return extension +} + +func getKindExtension(kind string) (extension string){ +switch strings.ToLower(kind) { +case NODE_JS: +extension = NODE_JS_EXT +case PYTHON: +extension = PYTHON_EXT +case SWIFT: +fallthrough +case PHP: +extension = "." + kind +} + +return extension +} + +func saveCode(action whisk.Action, filename string) (err error) { +var code string +var kind string +var exec whisk.Exec + +exec = *action.Exec +kind = strings.Split(exec.Kind, ":")[0] + +if strings.ToLower(kind) == BLACKBOX { +return cannotSaveImageError() +} else if strings.ToLower(kind) == SEQUENCE { +return cannotSaveSequenceError() +} + +if exec.Code == nil { +code = "" +} else { +code = *exec.Code +} + +if *exec.Binary { +decoded, _ := base64.StdEncoding.DecodeString(code) +code = string(decoded) + +if len(filename) == 0 { +filename = action.Name + getBinaryKindExtension(kind) +} +} else { +if len(filename) == 0 { +filename = action.Name + getKindExtension(kind) +} +} Review comment: not 100% sure, but it seems that an npm .zip action won't be handled correctly. a npm .zip file is not an attachment, so the exec.binary will be `false`; also the associated exec.code is encoded binary which will need decoding back into the binary prior to writing out to a file. 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] dubeejw commented on issue #2847: Treat action code as attachments for created/updated actions
dubeejw commented on issue #2847: Treat action code as attachments for created/updated actions URL: https://github.com/apache/incubator-openwhisk/pull/2847#issuecomment-337052534 whisk.core.database.test.CacheConcurrencyTests > the cache should support concurrent CRUD without bogus residual cache entries, iter 1 FAILED Failure is a result of parallel action gets with updates. I think this test fails because there is now an extra database write for the attachment that causes the action get to fail as it does not have the correct document revision. 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 commented on a change in pull request #2544: Allow CLI to Save Code from Action
mdeuser commented on a change in pull request #2544: Allow CLI to Save Code from Action URL: https://github.com/apache/incubator-openwhisk/pull/2544#discussion_r144969129 ## File path: tools/cli/go-whisk-cli/commands/action.go ## @@ -495,6 +513,87 @@ func getExec(args []string, params ActionFlags) (*whisk.Exec, error) { return exec, nil } +func getBinaryKindExtension(kind string) (extension string){ +switch strings.ToLower(kind) { +case JAVA: +extension = JAVA_EXT +default: +extension = ZIP_EXT +} + +return extension +} + +func getKindExtension(kind string) (extension string){ +switch strings.ToLower(kind) { +case NODE_JS: +extension = NODE_JS_EXT +case PYTHON: +extension = PYTHON_EXT +case SWIFT: +fallthrough +case PHP: +extension = "." + kind +} + +return extension +} + +func saveCode(action whisk.Action, filename string) (err error) { +var code string +var kind string +var exec whisk.Exec + +exec = *action.Exec +kind = strings.Split(exec.Kind, ":")[0] + +if strings.ToLower(kind) == BLACKBOX { +return cannotSaveImageError() +} else if strings.ToLower(kind) == SEQUENCE { +return cannotSaveSequenceError() +} + +if exec.Code == nil { +code = "" +} else { +code = *exec.Code +} + +if *exec.Binary { +decoded, _ := base64.StdEncoding.DecodeString(code) +code = string(decoded) + +if len(filename) == 0 { +filename = action.Name + getBinaryKindExtension(kind) +} +} else { +if len(filename) == 0 { +filename = action.Name + getKindExtension(kind) +} +} + +if exists, err := FileExists(filename); err != nil { +return err +} else if exists { +return fileExistsError(filename) +} + +if err := writeFile(filename, code); err != nil { +return err +} + +pwd, err := os.Getwd() +if err != nil { +return err Review comment: log an error message indicating that it was the `os.Getwd` function that failed 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 commented on a change in pull request #2544: Allow CLI to Save Code from Action
mdeuser commented on a change in pull request #2544: Allow CLI to Save Code from Action URL: https://github.com/apache/incubator-openwhisk/pull/2544#discussion_r144975374 ## File path: tools/cli/go-whisk-cli/commands/util.go ## @@ -851,6 +858,44 @@ func readFile(filename string) (string, error) { return string(file), nil } +func writeFile(filename string, content string) (error) { +file, err := os.Create(filename) + +if err != nil { +whisk.Debug(whisk.DbgError, "os.Create(%s) error: %#v\n", filename, err) +errMsg := wski18n.T("Cannot create file '{{.name}}': {{.err}}", +map[string]interface{}{"name": filename, "err": err}) +whiskErr := whisk.MakeWskError(errors.New(errMsg), whisk.EXIT_CODE_ERR_USAGE, whisk.DISPLAY_MSG, +whisk.DISPLAY_USAGE) +return whiskErr +} + +defer file.Close() + +file.WriteString(content) Review comment: Check the error that WriteString returns ---- 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] dubeejw commented on issue #2847: Treat action code as attachments for created/updated actions
dubeejw commented on issue #2847: Treat action code as attachments for created/updated actions URL: https://github.com/apache/incubator-openwhisk/pull/2847#issuecomment-337048815 Can expand test from https://github.com/apache/incubator-openwhisk/pull/2855 after that one is merged. 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] dubeejw commented on issue #2847: Treat action code as attachments for created/updated actions
dubeejw commented on issue #2847: Treat action code as attachments for created/updated actions URL: https://github.com/apache/incubator-openwhisk/pull/2847#issuecomment-337048815 Can use test from https://github.com/apache/incubator-openwhisk/pull/2855 after that one is merged. 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] dubeejw commented on issue #2832: Do not cache invalidate when adding a DB attachment
dubeejw commented on issue #2832: Do not cache invalidate when adding a DB attachment URL: https://github.com/apache/incubator-openwhisk/pull/2832#issuecomment-337043697 PG2 2173 ? 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 commented on a change in pull request #2544: Allow CLI to Save Code from Action
mdeuser commented on a change in pull request #2544: Allow CLI to Save Code from Action URL: https://github.com/apache/incubator-openwhisk/pull/2544#discussion_r144969129 ## File path: tools/cli/go-whisk-cli/commands/action.go ## @@ -495,6 +513,87 @@ func getExec(args []string, params ActionFlags) (*whisk.Exec, error) { return exec, nil } +func getBinaryKindExtension(kind string) (extension string){ +switch strings.ToLower(kind) { +case JAVA: +extension = JAVA_EXT +default: +extension = ZIP_EXT +} + +return extension +} + +func getKindExtension(kind string) (extension string){ +switch strings.ToLower(kind) { +case NODE_JS: +extension = NODE_JS_EXT +case PYTHON: +extension = PYTHON_EXT +case SWIFT: +fallthrough +case PHP: +extension = "." + kind +} + +return extension +} + +func saveCode(action whisk.Action, filename string) (err error) { +var code string +var kind string +var exec whisk.Exec + +exec = *action.Exec +kind = strings.Split(exec.Kind, ":")[0] + +if strings.ToLower(kind) == BLACKBOX { +return cannotSaveImageError() +} else if strings.ToLower(kind) == SEQUENCE { +return cannotSaveSequenceError() +} + +if exec.Code == nil { +code = "" +} else { +code = *exec.Code +} + +if *exec.Binary { +decoded, _ := base64.StdEncoding.DecodeString(code) +code = string(decoded) + +if len(filename) == 0 { +filename = action.Name + getBinaryKindExtension(kind) +} +} else { +if len(filename) == 0 { +filename = action.Name + getKindExtension(kind) +} +} + +if exists, err := FileExists(filename); err != nil { +return err +} else if exists { +return fileExistsError(filename) +} + +if err := writeFile(filename, code); err != nil { +return err +} + +pwd, err := os.Getwd() +if err != nil { +return err Review comment: also log an error message indicating that it was the `os.Getwd` function that failed 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 commented on a change in pull request #2544: Allow CLI to Save Code from Action
mdeuser commented on a change in pull request #2544: Allow CLI to Save Code from Action URL: https://github.com/apache/incubator-openwhisk/pull/2544#discussion_r144967923 ## File path: tools/cli/go-whisk-cli/commands/action.go ## @@ -495,6 +513,87 @@ func getExec(args []string, params ActionFlags) (*whisk.Exec, error) { return exec, nil } +func getBinaryKindExtension(kind string) (extension string){ +switch strings.ToLower(kind) { +case JAVA: +extension = JAVA_EXT +default: +extension = ZIP_EXT +} + +return extension +} + +func getKindExtension(kind string) (extension string){ +switch strings.ToLower(kind) { +case NODE_JS: +extension = NODE_JS_EXT +case PYTHON: +extension = PYTHON_EXT +case SWIFT: +fallthrough +case PHP: +extension = "." + kind +} + +return extension +} + +func saveCode(action whisk.Action, filename string) (err error) { +var code string +var kind string +var exec whisk.Exec + +exec = *action.Exec +kind = strings.Split(exec.Kind, ":")[0] Review comment: nit. possible confusion by using `kind` as the variable name when it really holds the runtime name and not the entire kind value. i.e. `nodejs:6` is the action's exec.kind value; this `kind` var will be set to `nodejs`. 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 commented on a change in pull request #2544: Allow CLI to Save Code from Action
mdeuser commented on a change in pull request #2544: Allow CLI to Save Code from Action URL: https://github.com/apache/incubator-openwhisk/pull/2544#discussion_r144965678 ## File path: tests/src/test/scala/whisk/core/cli/test/WskBasicUsageTests.scala ## @@ -811,6 +813,81 @@ class WskBasicUsageTests extends TestHelpers with WskTestHelpers { stdoutNoDescOrParams should include regex (s"(?i)action /${namespace}/${actNameNoDescOrParams}\\s*\\(parameters: none defined\\)") } + it should "save action code to file" in withAssetCleaner(wskprops) { (wp, assetHelper) => +val name = "saveAction" +val seqName = "seqName" +val dockerName = "dockerName" +val containerName = s"bogus${Random.alphanumeric.take(16).mkString.toLowerCase}" +val saveName = s"save-as-$name.js" +val badSaveName = s"bad-directory${File.separator}$saveName" + +// Test for successful --save +assetHelper.withCleaner(wsk.action, name) { (action, _) => + action.create(name, defaultAction) +} + +val saveMsg = wsk.action.get(name, save = Some(true)).stdout + +saveMsg should include(s"saved action code to ") + +val savePath = saveMsg.split("ok: saved action code to ")(1).trim() +val saveFile = new File(savePath); + +try { + saveFile.exists shouldBe true + + // Test for failure saving file when it already exist + wsk.action.get(name, save = Some(true), expectedExitCode = MISUSE_EXIT).stderr should include( +s"The file '$name.js' already exists") +} finally { + saveFile.delete() +} + +// Test for successful --save-as +val saveAsMsg = wsk.action.get(name, saveAs = Some(saveName)).stdout + +saveAsMsg should include(s"saved action code to ") + +val saveAsPath = saveAsMsg.split("ok: saved action code to ")(1).trim() +val saveAsFile = new File(saveAsPath); + +try { + saveAsFile.exists shouldBe true + + // Test for failure saving file when it already exist + wsk.action.get(name, saveAs = Some(saveName), expectedExitCode = MISUSE_EXIT).stderr should include( +s"The file '$saveName' already exists") +} finally { + saveAsFile.delete() +} + +// Test for failure when using an invalid filename +wsk.action.get(name, saveAs = Some(badSaveName), expectedExitCode = MISUSE_EXIT).stderr should include( + s"Cannot create file '$badSaveName'") + +// Test for failure saving Docker images +assetHelper.withCleaner(wsk.action, dockerName) { (action, _) => + action.create(dockerName, None, docker = Some(containerName)) +} + +wsk.action.get(dockerName, save = Some(true), expectedExitCode = MISUSE_EXIT).stderr should include( + "Cannot save Docker images") + +wsk.action.get(dockerName, saveAs = Some(dockerName), expectedExitCode = MISUSE_EXIT).stderr should include( + "Cannot save Docker images") + +// Tes for failure saving sequences +assetHelper.withCleaner(wsk.action, seqName) { (action, _) => + action.create(seqName, Some(name), kind = Some("sequence")) +} + +wsk.action.get(seqName, save = Some(true), expectedExitCode = MISUSE_EXIT).stderr should include( + "Cannot save action sequence") Review comment: can the CLI handle saving an action with a 50M npm .zip file? 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 commented on a change in pull request #2544: Allow CLI to Save Code from Action
mdeuser commented on a change in pull request #2544: Allow CLI to Save Code from Action URL: https://github.com/apache/incubator-openwhisk/pull/2544#discussion_r144964356 ## File path: tools/cli/go-whisk-cli/commands/action.go ## @@ -495,6 +513,87 @@ func getExec(args []string, params ActionFlags) (*whisk.Exec, error) { return exec, nil } +func getBinaryKindExtension(kind string) (extension string){ +switch strings.ToLower(kind) { +case JAVA: +extension = JAVA_EXT +default: +extension = ZIP_EXT +} + +return extension +} + +func getKindExtension(kind string) (extension string){ +switch strings.ToLower(kind) { +case NODE_JS: +extension = NODE_JS_EXT +case PYTHON: +extension = PYTHON_EXT +case SWIFT: +fallthrough +case PHP: +extension = "." + kind +} + +return extension +} + +func saveCode(action whisk.Action, filename string) (err error) { +var code string +var kind string +var exec whisk.Exec + +exec = *action.Exec +kind = strings.Split(exec.Kind, ":")[0] + +if strings.ToLower(kind) == BLACKBOX { +return cannotSaveImageError() +} else if strings.ToLower(kind) == SEQUENCE { +return cannotSaveSequenceError() +} + +if exec.Code == nil { +code = "" +} else { +code = *exec.Code +} + +if *exec.Binary { +decoded, _ := base64.StdEncoding.DecodeString(code) +code = string(decoded) + +if len(filename) == 0 { +filename = action.Name + getBinaryKindExtension(kind) +} +} else { +if len(filename) == 0 { +filename = action.Name + getKindExtension(kind) +} +} Review comment: not 100% sure, but it seems that an npm .zip action won't be handled correctly. a npm .zip file is not an attachment, so the exec.binary will be `false`; also the associated exec.code is encoded binary which will need decoding back into the binary prior to writing out to a file. 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] dgrove-oss commented on issue #2828: option to use docker pause/unpause instead of docker-runc pause/unpause
dgrove-oss commented on issue #2828: option to use docker pause/unpause instead of docker-runc pause/unpause URL: https://github.com/apache/incubator-openwhisk/pull/2828#issuecomment-337033927 squashed down to single commit. Please merge once Travis verifies the squashed commit. 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 commented on a change in pull request #2544: Allow CLI to Save Code from Action
mdeuser commented on a change in pull request #2544: Allow CLI to Save Code from Action URL: https://github.com/apache/incubator-openwhisk/pull/2544#discussion_r144949391 ## File path: tests/src/test/scala/whisk/core/cli/test/WskBasicUsageTests.scala ## @@ -1347,8 +1424,8 @@ class WskBasicUsageTests extends TestHelpers with WskTestHelpers { it should "reject commands that are executed with a missing or invalid parameter or annotation file" in { val emptyFile = TestUtils.getTestActionFilename("emtpy.js") val missingFile = "notafile" -val emptyFileMsg = s"File '$emptyFile' is not a valid file or it does not exist" -val missingFileMsg = s"File '$missingFile' is not a valid file or it does not exist" +val emptyFileMsg = s"The file '$emptyFile' does not exist" Review comment: Incorrect error message. An empty file actually exists. ---- 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 commented on a change in pull request #2544: Allow CLI to Save Code from Action
mdeuser commented on a change in pull request #2544: Allow CLI to Save Code from Action URL: https://github.com/apache/incubator-openwhisk/pull/2544#discussion_r144948805 ## File path: tests/src/test/scala/whisk/core/cli/test/WskBasicUsageTests.scala ## @@ -811,6 +813,81 @@ class WskBasicUsageTests extends TestHelpers with WskTestHelpers { stdoutNoDescOrParams should include regex (s"(?i)action /${namespace}/${actNameNoDescOrParams}\\s*\\(parameters: none defined\\)") } + it should "save action code to file" in withAssetCleaner(wskprops) { (wp, assetHelper) => +val name = "saveAction" +val seqName = "seqName" +val dockerName = "dockerName" +val containerName = s"bogus${Random.alphanumeric.take(16).mkString.toLowerCase}" +val saveName = s"save-as-$name.js" +val badSaveName = s"bad-directory${File.separator}$saveName" + +// Test for successful --save +assetHelper.withCleaner(wsk.action, name) { (action, _) => + action.create(name, defaultAction) +} + +val saveMsg = wsk.action.get(name, save = Some(true)).stdout + +saveMsg should include(s"saved action code to ") + +val savePath = saveMsg.split("ok: saved action code to ")(1).trim() +val saveFile = new File(savePath); + +try { + saveFile.exists shouldBe true + + // Test for failure saving file when it already exist + wsk.action.get(name, save = Some(true), expectedExitCode = MISUSE_EXIT).stderr should include( +s"The file '$name.js' already exists") +} finally { + saveFile.delete() +} + +// Test for successful --save-as +val saveAsMsg = wsk.action.get(name, saveAs = Some(saveName)).stdout + +saveAsMsg should include(s"saved action code to ") + +val saveAsPath = saveAsMsg.split("ok: saved action code to ")(1).trim() +val saveAsFile = new File(saveAsPath); + +try { + saveAsFile.exists shouldBe true + + // Test for failure saving file when it already exist + wsk.action.get(name, saveAs = Some(saveName), expectedExitCode = MISUSE_EXIT).stderr should include( +s"The file '$saveName' already exists") +} finally { + saveAsFile.delete() +} + +// Test for failure when using an invalid filename +wsk.action.get(name, saveAs = Some(badSaveName), expectedExitCode = MISUSE_EXIT).stderr should include( + s"Cannot create file '$badSaveName'") + +// Test for failure saving Docker images +assetHelper.withCleaner(wsk.action, dockerName) { (action, _) => + action.create(dockerName, None, docker = Some(containerName)) +} + +wsk.action.get(dockerName, save = Some(true), expectedExitCode = MISUSE_EXIT).stderr should include( + "Cannot save Docker images") + +wsk.action.get(dockerName, saveAs = Some(dockerName), expectedExitCode = MISUSE_EXIT).stderr should include( + "Cannot save Docker images") + +// Tes for failure saving sequences +assetHelper.withCleaner(wsk.action, seqName) { (action, _) => + action.create(seqName, Some(name), kind = Some("sequence")) +} + +wsk.action.get(seqName, save = Some(true), expectedExitCode = MISUSE_EXIT).stderr should include( + "Cannot save action sequence") Review comment: nit. error message consistency. `Cannot save Docker images` is plural. `Cannot save action sequence` is singular and does not read quite as well. 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 #2859: LoadBalancerService does not consider concurrent requests when scheduling
vvraskin commented on issue #2859: LoadBalancerService does not consider concurrent requests when scheduling URL: https://github.com/apache/incubator-openwhisk/issues/2859#issuecomment-337006404 Good catch. I will try to reproduce it later, but I think you are right, the fact that we read async, doesn't allow us to see the update by the second call of `schedule`. Since we only write to the local instances, we could consider changing read to be synchronous, need to measure the performance overhead though. According to akka doc the use case you are showing should be supported: > You will always read your own writes. For example if you send a Update message followed by a Get of the same key the Get will retrieve the change that was performed by the preceding Update message. 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] bwmcadams commented on a change in pull request #2802: Akka Usage Cleanup for Actor State safety
bwmcadams commented on a change in pull request #2802: Akka Usage Cleanup for Actor State safety URL: https://github.com/apache/incubator-openwhisk/pull/2802#discussion_r144941287 ## File path: core/invoker/src/main/scala/whisk/core/containerpool/ContainerPool.scala ## @@ -115,26 +115,31 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef, // Container is free to take more work case NeedWork(data: WarmedData) => - freePool.update(sender(), data) - busyPool.remove(sender()).foreach(_ => feed ! MessageFeed.Processed) + freePool = freePool + (sender() -> data) + busyPool = busyPool - sender() + busyPool.foreach { _ => Review comment: oof. that was a stupid bug, but explains the behavior I was seeing in my failed tests. (the foreach wrapper in the original code handled the `if contains`) 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] bwmcadams commented on a change in pull request #2802: Akka Usage Cleanup for Actor State safety
bwmcadams commented on a change in pull request #2802: Akka Usage Cleanup for Actor State safety URL: https://github.com/apache/incubator-openwhisk/pull/2802#discussion_r144941287 ## File path: core/invoker/src/main/scala/whisk/core/containerpool/ContainerPool.scala ## @@ -115,26 +115,31 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef, // Container is free to take more work case NeedWork(data: WarmedData) => - freePool.update(sender(), data) - busyPool.remove(sender()).foreach(_ => feed ! MessageFeed.Processed) + freePool = freePool + (sender() -> data) + busyPool = busyPool - sender() + busyPool.foreach { _ => Review comment: oof. that was a stupid bug, but explains the behavior I was seeing in my failed tests. 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 commented on a change in pull request #2589: Add the fundamental framework of REST invocation for test cases
houshengbo commented on a change in pull request #2589: Add the fundamental framework of REST invocation for test cases URL: https://github.com/apache/incubator-openwhisk/pull/2589#discussion_r144934598 ## File path: tests/src/test/scala/common/rest/WskRest.scala ## @@ -0,0 +1,1482 @@ +/* + * 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 common.rest + +import java.io.File +import java.time.Clock +import java.time.Instant +import java.util.Base64 + +import org.apache.commons.io.FileUtils +import org.scalatest.Matchers +import org.scalatest.FlatSpec +import org.scalatest.concurrent.ScalaFutures +import org.scalatest.time.Span.convertDurationToSpan + +import scala.Left +import scala.Right +import scala.collection.JavaConversions.mapAsJavaMap +import scala.collection.mutable.Buffer +import scala.collection.immutable.Seq +import scala.concurrent.duration.Duration +import scala.concurrent.duration.DurationInt +import scala.concurrent.Future +import scala.language.postfixOps +import scala.util.Failure +import scala.util.Success +import scala.util.Try +import scala.util.{Failure, Success} + +import akka.http.scaladsl.model.StatusCode +import akka.http.scaladsl.model.StatusCodes.Accepted +import akka.http.scaladsl.model.StatusCodes.NotFound +import akka.http.scaladsl.model.StatusCodes.BadRequest +import akka.http.scaladsl.model.StatusCodes.OK +import akka.http.scaladsl.model.HttpRequest +import akka.http.scaladsl.model.HttpMethod +import akka.http.scaladsl.model.HttpResponse +import akka.http.scaladsl.model.headers.Authorization +import akka.http.scaladsl.model.HttpEntity +import akka.http.scaladsl.model.ContentTypes +import akka.http.scaladsl.Http + +import akka.http.scaladsl.model.headers.BasicHttpCredentials +import akka.http.scaladsl.model.Uri +import akka.http.scaladsl.model.Uri.Path + +import akka.http.scaladsl.model.HttpMethods.DELETE +import akka.http.scaladsl.model.HttpMethods.GET +import akka.http.scaladsl.model.HttpMethods.POST +import akka.http.scaladsl.model.HttpMethods.PUT + +import akka.stream.ActorMaterializer + +import spray.json._ +import spray.json.DefaultJsonProtocol._ +import spray.json.JsObject +import spray.json.JsValue +import spray.json.pimpString + +import common._ +import common.BaseDeleteFromCollection +import common.BaseListOrGetFromCollection +import common.HasActivation +import common.RunWskCmd +import common.TestUtils +import common.TestUtils.SUCCESS_EXIT +import common.TestUtils.DONTCARE_EXIT +import common.TestUtils.ANY_ERROR_EXIT +import common.TestUtils.DONTCARE_EXIT +import common.TestUtils.RunResult +import common.WaitFor +import common.WhiskProperties +import common.WskActorSystem +import common.WskProps + +import whisk.core.entity.ByteSize +import whisk.utils.retry + +class WskRest() extends RunWskRestCmd with BaseWsk { + override implicit val action = new WskRestAction + override implicit val trigger = new WskRestTrigger + override implicit val rule = new WskRestRule + override implicit val activation = new WskRestActivation + override implicit val pkg = new WskRestPackage + override implicit val namespace = new WskRestNamespace + override implicit val api = new WskRestApi +} + +trait ListOrGetFromCollectionRest extends BaseListOrGetFromCollection { + self: RunWskRestCmd => + + /** + * List entities in collection. + * + * @param namespace (optional) if specified must be fully qualified namespace + * @param expectedExitCode (optional) the expected exit code for the command + * if the code is anything but DONTCARE_EXIT, assert the code is as expected + */ + override def list(namespace: Option[String] = None, +limit: Option[Int] = None, +nameSort: Option[Boolean] = None, +expectedExitCode: Int = OK.intValue)(implicit wp: WskProps): RestResult = { +val NS = namespace map { ns => Review comment: I will get rid of val NS. 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 th
[GitHub] dubeejw commented on a change in pull request #2855: Cache database attachments
dubeejw commented on a change in pull request #2855: Cache database attachments URL: https://github.com/apache/incubator-openwhisk/pull/2855#discussion_r144642462 ## File path: common/scala/src/main/scala/whisk/core/database/DocumentFactory.scala ## @@ -245,26 +261,35 @@ trait DocumentFactory[W] extends MultipleReadersSingleWriterCache[W, DocInfo] { } } - def getAttachment[Wsuper >: W](db: ArtifactStore[Wsuper], - doc: DocInfo, - attachmentName: String, - outputStream: OutputStream)(implicit transid: TransactionId): Future[Unit] = { + def getAttachment[Wsuper >: W]( +db: ArtifactStore[Wsuper], +entity: W, +doc: DocInfo, +attachmentName: String, +outputStream: OutputStream, +postProcess: Option[PostProcessEntity[W]] = None)(implicit transid: TransactionId, mw: Manifest[W]): Future[W] = { implicit val ec = db.executionContext +implicit val notifier: Option[CacheChangeNotification] = None Review comment: Cache invalidation callback should probably be assigned to the actual callback 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] tysonnorris commented on issue #2856: How do you limit the concurrent activations with multiple Controller?
tysonnorris commented on issue #2856: How do you limit the concurrent activations with multiple Controller? URL: https://github.com/apache/incubator-openwhisk/issues/2856#issuecomment-336979661 @hykych @vvraskin I just logged #2859 - is this possibly related? 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 issue #2859: LoadBalancerService does not consider concurrent requests when scheduling
tysonnorris opened a new issue #2859: LoadBalancerService does not consider concurrent requests when scheduling URL: https://github.com/apache/incubator-openwhisk/issues/2859 Under concurrent requests to controller, LoadBalancerService will not consider concurrent requests when scheduling to an invoker. For example, consider `LoadBalacnerServiceObjectTests.scala`, with the following test ``` it should "consider load immediately and jump to next invoker when overloaded" in { val invokerCount = 10 val hash = 2 val targetInvoker = hash % invokerCount val invs = invokers(invokerCount).updated(targetInvoker, (InstanceId(targetInvoker), Healthy, 0)) val step = hashInto(LoadBalancerService.pairwiseCoprimeNumbersUntil(invokerCount), hash) LoadBalancerService.schedule(invs, 1, hash) shouldBe Some(InstanceId((hash + 0) % invs.size)) LoadBalancerService.schedule(invs, 1, hash) shouldBe Some(InstanceId((hash + 1) % invs.size)) } ``` We have: * set the existing load to 0 to start with * set the invokerBusyThreshold to 1 on `scheduler()` invocations * schedule one activation with hash=2 * schedule second activation with hash=2 In this case, but activations are scheduled to InstanceId(2), but the second should be scheduled to next invoker since busy threshold is exceeded. I believe this is related to the changing of `LoadbalancerData.activationCountPerInvoker()` from sync to async. Since LoadBalancerService is processed with multiple threads, the updates to the LoadBalancerData are not seen immediately by other threads. 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] csantanapr closed pull request #40: Revert "Add the support of certificate checking"
csantanapr closed pull request #40: Revert "Add the support of certificate checking" URL: https://github.com/apache/incubator-openwhisk-client-go/pull/40 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] csantanapr commented on issue #40: Revert "Add the support of certificate checking"
csantanapr commented on issue #40: Revert "Add the support of certificate checking" URL: https://github.com/apache/incubator-openwhisk-client-go/pull/40#issuecomment-336975635 @houshengbo @mrutkows We are reverting the changes from #39 There was an error introduce, that required a cert and key to be configure for secure https hosts I think the fix is removing the `else` here https://github.com/apache/incubator-openwhisk-client-go/blob/master/whisk/client.go#L160-L162 as the missing a cert and key for secure https hosts is a valid configuration. Also this change should go first to main repo `client.go` and then ported/sync back to this repo 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] mrutkows commented on issue #608: Invalid Runtime not detected / no error generated
mrutkows commented on issue #608: Invalid Runtime not detected / no error generated URL: https://github.com/apache/incubator-openwhisk-wskdeploy/issues/608#issuecomment-336966727 In addition, issue #306 lists what we SHOULD do (copied below), but I am not confident we test for many of these conditions NOR do we have features opened for those that require new code. 1. verify the kind (if it has one) a valid extension ":default" as part if the string OR 2. verify it has a valid version number, for example ":6" or ":3" 3.Notify user with a specific error saying "Specified runtime (kind) is not recognized as being valid" along with the line number in the manifest where this was parsed. 4.Allow simple "java" as a value and assume we default to "latest", that is we treat it like "java:default" instead. 5.**FEATURE**: if the user specifies a valid kind runtime:nodejs:4 but the version is not recognized we should look to interactively prompt the user asking if they want to continue with the latest "default" for that runtime kind (PLEASE OPEN A NEW ISSUE FOR THIS INTERACTIVE MODE). 6.**FEATURE**: we should perhaps call the CLI to list the known runtimes and not hardcode a list at all and use the JSON response to derive a list of valid runtime (kinds) AND we should provide WARNINGS to the user of the runtimes are marked "deprecated". (PLEASE OPEN A NEW ISSUE FOR FEATURE). -and if we do not find a runtime key at all in the manifest we should of course still "default" to "nodejs:default", BUT WE SHOULD PRESENT A WARNING in the output to the user that we are making this assumption. ---- 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] mrutkows commented on issue #608: Invalid Runtime not detected / no error generated
mrutkows commented on issue #608: Invalid Runtime not detected / no error generated URL: https://github.com/apache/incubator-openwhisk-wskdeploy/issues/608#issuecomment-336966727 In addition, issue #306 lists what we SHOULD do (copied below), but I am not confident we test for many of these conditions NOR do we have features opened for those that require new code. 1. verify the kind (if it has one) a valid extension ":default" as part if the string OR 2. verify it has a valid version number, for example ":6" or ":3" 3.Notify user with a specific error saying "Specified runtime (kind) is not recognized as being valid" along with the line number in the manifest where this was parsed. 4.Allow simple "java" as a value and assume we default to "latest", that is we treat it like "java:default" instead. 5.FEATURE: if the user specifies a valid kind runtime:nodejs:4 but the version is not recognized we should look to interactively prompt the user asking if they want to continue with the latest "default" for that runtime kind (PLEASE OPEN A NEW ISSUE FOR THIS INTERACTIVE MODE). 6.FEATURE: we should perhaps call the CLI to list the known runtimes and not hardcode a list at all and use the JSON response to derive a list of valid runtime (kinds) AND we should provide WARNINGS to the user of the runtimes are marked "deprecated". (PLEASE OPEN A NEW ISSUE FOR FEATURE). -and if we do not find a runtime key at all in the manifest we should of course still "default" to "nodejs:default", BUT WE SHOULD PRESENT A WARNING in the output to the user that we are making this assumption. ---- 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] mrutkows commented on issue #608: Invalid Runtime not detected / no error generated
mrutkows commented on issue #608: Invalid Runtime not detected / no error generated URL: https://github.com/apache/incubator-openwhisk-wskdeploy/issues/608#issuecomment-336966727 In addition, issue #306 lists what we SHOULD do (copied below), but I am not confident we test for many of these conditions NOR do we have features opened for those that require new code. 1. verify the kind (if it has one) a valid extension ":default" as part if the string OR 2. verify it has a valid version number, for example ":6" or ":3" -Notify user with a specific error saying "Specified runtime (kind) is not recognized as being valid" along with the line number in the manifest where this was parsed. -Allow simple "java" as a value and assume we default to "latest", that is we treat it like "java:default" instead. 3.FEATURE: if the user specifies a valid kind runtime:nodejs:4 but the version is not recognized we should look to interactively prompt the user asking if they want to continue with the latest "default" for that runtime kind (PLEASE OPEN A NEW ISSUE FOR THIS INTERACTIVE MODE). 4.FEATURE: we should perhaps call the CLI to list the known runtimes and not hardcode a list at all and use the JSON response to derive a list of valid runtime (kinds) AND we should provide WARNINGS to the user of the runtimes are marked "deprecated". (PLEASE OPEN A NEW ISSUE FOR FEATURE). -and if we do not find a runtime key at all in the manifest we should of course still "default" to "nodejs:default", BUT WE SHOULD PRESENT A WARNING in the output to the user that we are making this assumption. ---- 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] mrutkows commented on issue #608: Invalid Runtime not detected / no error generated
mrutkows commented on issue #608: Invalid Runtime not detected / no error generated URL: https://github.com/apache/incubator-openwhisk-wskdeploy/issues/608#issuecomment-336966727 In addition, issue #306 lists what we SHOULD do (copied below), but I am not confident we test for many of these conditions: ``` verify the kind (if it has one) a valid extension ":default" as part if the string OR verify it has a valid version number, for example ":6" or ":3" Notify user with a specific error saying "Specified runtime (kind) is not recognized as being valid" along with the line number in the manifest where this was parsed. Allow simple "java" as a value and assume we default to "latest", that is we treat it like "java:default" instead. FEATURE: if the user specifies a valid kind runtime:nodejs:4 but the version is not recognized we should look to interactively prompt the user asking if they want to continue with the latest "default" for that runtime kind (PLEASE OPEN A NEW ISSUE FOR THIS INTERACTIVE MODE). FEATURE: we should perhaps call the CLI to list the known runtimes and not hardcode a list at all and use the JSON response to derive a list of valid runtime (kinds) AND we should provide WARNINGS to the user of the runtimes are marked "deprecated". (PLEASE OPEN A NEW ISSUE FOR FEATURE). and if we do not find a runtime key at all in the manifest we should of course still "default" to "nodejs:default", BUT WE SHOULD PRESENT A WARNING in the output to the user that we are making this assumption. ``` ---- 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] mrutkows opened a new issue #608: Invalid Runtime not detected / no error generated
mrutkows opened a new issue #608: Invalid Runtime not detected / no error generated URL: https://github.com/apache/incubator-openwhisk-wskdeploy/issues/608 manifest_parser_test includes a unit test "TestComposeActionsForInvalidRuntime" The actual test is commented out and it simply outputs a error (meaning the invalid runtime was not reported as an error). The function contains several TODO that suggest issue 307 will address this ... HOWEVER issue 307 (and ref. issue 306) are both closed. There still is not "InvalidRuntime" error... we need such a named error to be thrown if the manifest has this field set (and it is not inferred) to an unknown value. 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] dubeejw opened a new pull request #40: Revert "Add the support of certificate checking"
dubeejw opened a new pull request #40: Revert "Add the support of certificate checking" URL: https://github.com/apache/incubator-openwhisk-client-go/pull/40 Reverts apache/incubator-openwhisk-client-go#39 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] mrutkows commented on issue #116: discussion: parameters in action sequence
mrutkows commented on issue #116: discussion: parameters in action sequence URL: https://github.com/apache/incubator-openwhisk/issues/116#issuecomment-336948883 The "wskdeploy" grammar/utility support declaration of input and output types (including more robust types that have schema such as JSON) [incubator-openwhisk-wskdeploy](https://github.com/apache/incubator-openwhisk-wskdeploy). The tool currently performs client-side validation of parameter values to declared (or inferred) types and we allows composition grammar as well. It would be great to agree on a grammar and means to insert this (perhaps as annotations) and perform server-side validation (using similar code as to the client-side). 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] mrutkows opened a new pull request #607: WIP: JSON Output support
mrutkows opened a new pull request #607: WIP: JSON Output support URL: https://github.com/apache/incubator-openwhisk-wskdeploy/pull/607 First part of PR we are "Updating Bad YAML tests to be more descriptive and efficient." 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 #2856: How do you limit the concurrent activations with multiple Controller?
vvraskin commented on issue #2856: How do you limit the concurrent activations with multiple Controller? URL: https://github.com/apache/incubator-openwhisk/issues/2856#issuecomment-336924888 Hi @hykych! basically there are two ways you deploy controllers: active-passive (one controller is active, another one becomes active if the first one fails), active-active (requests are dispatched for multiple controllers simultaneously). I think you mentioned the second case. So for that we use akka cluster to share the number of activations per namespace and activations per invoker across cluster members. You could look it up here: https://github.com/apache/incubator-openwhisk/blob/master/core/controller/src/main/scala/whisk/core/loadBalancer/DistributedLoadBalancerData.scala and here is where we enable this class: https://github.com/apache/incubator-openwhisk/blob/master/core/controller/src/main/scala/whisk/core/loadBalancer/LoadBalancerService.scala#L105 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] cbickel commented on issue #2665: Add ability to use all controllers round robin
cbickel commented on issue #2665: Add ability to use all controllers round robin URL: https://github.com/apache/incubator-openwhisk/pull/2665#issuecomment-336920701 PG3#1191 is ? 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] sven-lange-last commented on issue #2858: Make Docker action container cleanup during invoker startup / shutdown more robust
sven-lange-last commented on issue #2858: Make Docker action container cleanup during invoker startup / shutdown more robust URL: https://github.com/apache/incubator-openwhisk/pull/2858#issuecomment-336915644 PG 1 / 2143 running. 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] sven-lange-last commented on issue #2858: Make Docker action container cleanup during invoker startup / shutdown more robust
sven-lange-last commented on issue #2858: Make Docker action container cleanup during invoker startup / shutdown more robust URL: https://github.com/apache/incubator-openwhisk/pull/2858#issuecomment-336909014 @markusthoemmes please attend to this PR. 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] sven-lange-last opened a new pull request #2858: Make Docker action container cleanup during invoker startup / shutdown more robust
sven-lange-last opened a new pull request #2858: Make Docker action container cleanup during invoker startup / shutdown more robust URL: https://github.com/apache/incubator-openwhisk/pull/2858 Remove all Docker containers on the system that match the naming filter `wsk_` at startup and shutdown. In the past, only running containers matching the filter were removed - paused containers are considered to be running. The new strategy will also remove containers that are stopped or could never be started properly for any reason. From time to time, we see action containers that are not running but should be cleaned up. If Docker commands used to remove containers (`docker ps`, `docker-runc resume` and `docker rm`) do not return within 30 seconds during startup, abort startup and let Docker re-start the invoker container. We have seen systems with heavy load where Docker commands take too long to complete. In that situation, we need a defined behavior, i.e. re-start and re-try the cleanup. At the moment, an uncaught exception will end the main thread leaving the invoker inoperational. Also changed the invoker initialization sequence such that Docker action container cleanup is performed as soon as possible - in particular, before starting other tasks / actors. At the moment, a cleanup timeout would end the main thread while the activation message feed is already running and consuming messsages that will never be processed. 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 #2857: Emit metrics via kamon - wip
markusthoemmes commented on issue #2857: Emit metrics via kamon - wip URL: https://github.com/apache/incubator-openwhisk/pull/2857#issuecomment-336870752 Sorry oversaw the **WIP** tag, my bad. 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 #2857: Emit metrics via kamon - wip
markusthoemmes commented on a change in pull request #2857: Emit metrics via kamon - wip URL: https://github.com/apache/incubator-openwhisk/pull/2857#discussion_r144826997 ## File path: common/scala/src/main/scala/whisk/common/TransactionId.scala ## @@ -55,7 +55,14 @@ case class TransactionId private (meta: TransactionMetadata) extends AnyVal { */ def mark(from: AnyRef, marker: LogMarkerToken, message: String = "", logLevel: LogLevel = InfoLevel)( implicit logging: Logging) = { -logging.emit(logLevel, this, from, createMessageWithMarker(message, LogMarker(marker, deltaToStart))) + +if (meta.metricsLog) { + logging.emit(logLevel, this, from, createMessageWithMarker(message, LogMarker(marker, deltaToStart))) +} Review comment: This is a dangerous change in behavior. I'd rather have this flagged by additional information like so: ```scala if (message.nonEmpty) { val print = if(meta.metricsLog) createMessageWithMarker(message, LogMarker(marker, deltaToStart)) else message logging.emit(logLevel, this, from, print) } ``` Assuming that you still want the message to be printed if it was given by the developer (we should probably clean that up later and log the lines explicitly vs. implicitly through this class, but it keeps the changes small for now) 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 #2857: Emit metrics via kamon - wip
markusthoemmes commented on a change in pull request #2857: Emit metrics via kamon - wip URL: https://github.com/apache/incubator-openwhisk/pull/2857#discussion_r144827975 ## File path: common/scala/src/main/scala/whisk/common/Logging.scala ## @@ -178,6 +179,34 @@ object LogMarkerToken { } } +trait WhiskMetric {} + +case class WhiskCounterMetric(component: String, action: String) extends WhiskMetric {} + +case class WhiskHistogramMetric(component: String, action: String, value: Long) extends WhiskMetric + +object MetricEmitter { + + def getMetricName(component: String, action: String, isFailed: Boolean = false): String = { +if (isFailed) { + component + "_" + action + "_failed" +} else { + component + "_" + action +} + } + + def emitMetric(wm: WhiskMetric, isFailed: Boolean = false) = wm match { +case WhiskCounterMetric(component, action) => + Kamon.metrics +.counter(getMetricName(component, action, isFailed)) +.increment(1) +case WhiskHistogramMetric(component, action, value) => + Kamon.metrics +.histogram(getMetricName(component, action, isFailed)) +.record(value) + } Review comment: Why not have an overloaded version of `emitMetric` and get rid of the intermediary types + non-needed allocations? 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 #2857: Emit metrics via kamon - wip
markusthoemmes commented on a change in pull request #2857: Emit metrics via kamon - wip URL: https://github.com/apache/incubator-openwhisk/pull/2857#discussion_r144826997 ## File path: common/scala/src/main/scala/whisk/common/TransactionId.scala ## @@ -55,7 +55,14 @@ case class TransactionId private (meta: TransactionMetadata) extends AnyVal { */ def mark(from: AnyRef, marker: LogMarkerToken, message: String = "", logLevel: LogLevel = InfoLevel)( implicit logging: Logging) = { -logging.emit(logLevel, this, from, createMessageWithMarker(message, LogMarker(marker, deltaToStart))) + +if (meta.metricsLog) { + logging.emit(logLevel, this, from, createMessageWithMarker(message, LogMarker(marker, deltaToStart))) +} Review comment: This is a dangerous change in behavior. I'd rather have this flagged by additional information like so: ```scala if (message.nonEmpty) { val print = if(meta.metricsLog) createMessageWithMarker(message, LogMarker(marker, deltaToStart)) else message logging.emit(logLevel, this, from, print) } Assuming that you still want the message to be printed if it was given by the developer (we should probably clean that up later and log the lines explicitly vs. implicitly through this class, but it keeps the changes small for now) 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 #2857: Emit metrics via kamon - wip
markusthoemmes commented on a change in pull request #2857: Emit metrics via kamon - wip URL: https://github.com/apache/incubator-openwhisk/pull/2857#discussion_r144825903 ## File path: ansible/group_vars/all ## @@ -105,7 +105,11 @@ limits: controller: basePort: 10001 heap: "{{ controller_heap | default('2g') }}" - arguments: "{{ controller_arguments | default('') }}" + arguments: "{% if metrics.kamon == true %} +{{ controller_arguments + ' -Dkamon.statsd.hostname=' + metrics_kamon_statsd_host + ' -Dkamon.statsd.port=' + metrics_kamon_statsd_port | default('') }} + {% else %} +{{ controller_arguments | default('') }} + {% endif %}" Review comment: Let's fix these bits, I have an ENV->JVM System property adapter script stashed in another PR. I'll finish that off, then we can clean these up (We need system props for SPIs for example anyway). -------- 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] mhenke1 opened a new pull request #2857: Emit metrics via kamon
mhenke1 opened a new pull request #2857: Emit metrics via kamon URL: https://github.com/apache/incubator-openwhisk/pull/2857 This PR implements the capability to send metric information via Kamon (http://kamon.io/) to statsd (https://github.com/etsy/statsd) instead or additionally to the existing metrics information which is written into the system logs. Per default Kamon metrics are disabled and metrics as log information is enabled. The system behavior is expected to be the same as before, but can be changed via Ansible configuration by adding the following settings the "group_vars/all" file of an environment: - metrics_kamon -> enable metrics via kamon (true/false with default false) - metrics_kamon_statsd_host -> hostname (or ip address) of the statsd server - metrics_kamon_statsd_port-> port number of the statsd server - metrics_log: -> enable metrics via log file (true/false with default true) Example: ``` # Metrics configuration metrics_kamon: false metrics_kamon_statsd_host: '192.168.99.100' metrics_kamon_statsd_port: '8125' metrics_log: true ``` 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 closed pull request #2838: Add check to controller and invoker that required databases exist
markusthoemmes closed pull request #2838: Add check to controller and invoker that required databases exist URL: https://github.com/apache/incubator-openwhisk/pull/2838 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 #2802: Akka Usage Cleanup for Actor State safety
markusthoemmes commented on a change in pull request #2802: Akka Usage Cleanup for Actor State safety URL: https://github.com/apache/incubator-openwhisk/pull/2802#discussion_r144799933 ## File path: core/invoker/src/main/scala/whisk/core/containerpool/ContainerPool.scala ## @@ -115,26 +115,31 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef, // Container is free to take more work case NeedWork(data: WarmedData) => - freePool.update(sender(), data) - busyPool.remove(sender()).foreach(_ => feed ! MessageFeed.Processed) + freePool = freePool + (sender() -> data) + busyPool = busyPool - sender() + busyPool.foreach { _ => +feed ! MessageFeed.Processed + } // Container is prewarmed and ready to take work case NeedWork(data: PreWarmedData) => - prewarmedPool.update(sender(), data) + prewarmedPool = prewarmedPool + (sender() -> data) // Container got removed case ContainerRemoved => - freePool.remove(sender()) - busyPool.remove(sender()).foreach(_ => feed ! MessageFeed.Processed) + freePool = freePool - sender() + busyPool = busyPool - sender() + busyPool.foreach { _ => +feed ! MessageFeed.Processed + } Review comment: Same as above. 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 #2802: Akka Usage Cleanup for Actor State safety
markusthoemmes commented on a change in pull request #2802: Akka Usage Cleanup for Actor State safety URL: https://github.com/apache/incubator-openwhisk/pull/2802#discussion_r144799888 ## File path: core/invoker/src/main/scala/whisk/core/containerpool/ContainerPool.scala ## @@ -115,26 +115,31 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef, // Container is free to take more work case NeedWork(data: WarmedData) => - freePool.update(sender(), data) - busyPool.remove(sender()).foreach(_ => feed ! MessageFeed.Processed) + freePool = freePool + (sender() -> data) + busyPool = busyPool - sender() + busyPool.foreach { _ => Review comment: The logic here is wrong. Above code said: "remove this element, and if it was actually there, also send a message". So it's going to be more like: ```scala if(busyPool.contains(sender()) { busyPool = busyPool - sender() feed ! MessageFeed.Processed } ``` 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 closed pull request #2854: Improve garbage collector out-out-memory handling and debugging
markusthoemmes closed pull request #2854: Improve garbage collector out-out-memory handling and debugging URL: https://github.com/apache/incubator-openwhisk/pull/2854 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] jthomas commented on issue #76: Add type support for annotations and limits
jthomas commented on issue #76: Add type support for annotations and limits URL: https://github.com/apache/incubator-openwhisk-client-js/pull/76#issuecomment-336813254 LGTM. 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 #2792: Add a controller simulator to drive invoker benchmarks.
markusthoemmes commented on a change in pull request #2792: Add a controller simulator to drive invoker benchmarks. URL: https://github.com/apache/incubator-openwhisk/pull/2792#discussion_r144771977 ## File path: tools/benchmark/src/main/scala/whisk/core/benchmark/ControllerSimulator.scala ## @@ -0,0 +1,167 @@ +/* + * 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.benchmark + +import java.nio.charset.StandardCharsets +import java.time.Instant + +import akka.actor.{ActorSystem, Props} +import whisk.common.{AkkaLogging, Logging, TransactionId} +import whisk.core.WhiskConfig +import whisk.core.connector.{ActivationMessage, CompletionMessage, MessageFeed, MessagingProvider} +import whisk.core.database.NoDocumentException +import whisk.core.entity._ +import whisk.spi.SpiLoader + +import scala.collection.concurrent.TrieMap +import scala.concurrent.duration._ +import scala.concurrent.{Await, ExecutionContext, Future, Promise} + +object ControllerSimulator { + def between(start: Instant, end: Instant): Duration = +Duration.fromNanos(java.time.Duration.between(start, end).toNanos) + + def main(args: Array[String]): Unit = { +implicit val as: ActorSystem = ActorSystem() +implicit val ec: ExecutionContext = as.dispatcher +implicit val log: Logging = new AkkaLogging(as.log) +implicit val tid: TransactionId = TransactionId.controller + +val messageCount = sys.env.get("INVOCATIONS").map(_.toInt).getOrElse(10) +val controllerToSimulate = InstanceId(sys.env.get("CONTROLLER_ID").map(_.toInt).getOrElse(0)) +val invokerToUse = InstanceId(sys.env.get("INVOKER_ID").map(_.toInt).getOrElse(0)) + +val actionCode = sys.env.getOrElse("ACTION_CODE", "function main() { return {}; }") Review comment: I'm not too sure. As this takes the controller out of the game, I think this default makes the added overhead of the invoker apparent. It's also a default anybody can agree on, it's the smallest possible action. All other defaults we'll have to debate what a sensible default is. I'd leave it this way and then reconfigure on a per-test-basis if 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] hykych opened a new issue #2856: How do you limit the concurrent activations with multiple Controller?
hykych opened a new issue #2856: How do you limit the concurrent activations with multiple Controller? URL: https://github.com/apache/incubator-openwhisk/issues/2856 I noticed that you use the `activationByNamespaceId` in `LoadBalancerData` to count the concurrent activations. However, this is invalid with multiple Controller. Is there anything I didn't noticed or I made a mistake on that code snippet? 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] dubeejw commented on issue #2855: Cache database attachments
dubeejw commented on issue #2855: Cache database attachments URL: https://github.com/apache/incubator-openwhisk/pull/2855#issuecomment-336660943 PG3 1186 ? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rabbah commented on issue #116: discussion: parameters in action sequence
rabbah commented on issue #116: discussion: parameters in action sequence URL: https://github.com/apache/incubator-openwhisk/issues/116#issuecomment-336632898 [Composer](https://github.com/ibm-functions/composer/) provides capabilities for defining parameters that are scoped to individual actions, and also forwarding parameters around sequence segments. https://github.com/ibm-functions/composer. It is richer than the discussions we've had here in that it takes a different approach compared to `--sequence` which in contrast is very lightweight composition. We're continuing to develop this model of composition via _combinators_ and welcome feedback. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rabbah commented on issue #116: discussion: parameters in action sequence
rabbah commented on issue #116: discussion: parameters in action sequence URL: https://github.com/apache/incubator-openwhisk/issues/116#issuecomment-336632898 [Composer](https://github.com/ibm-functions/composer/) provides capabilities for defining parameters that are scoped to individual actions, and also forwarding parameters around sequence segments. https://github.com/ibm-functions/composer. It is richer than the discussions we've had here, and takes a different approach that `--sequence` which is contrast is very lightweight composition. We're continuing to develop this model of composition via _combinators_ and welcome feedback. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rabbah commented on issue #116: discussion: parameters in action sequence
rabbah commented on issue #116: discussion: parameters in action sequence URL: https://github.com/apache/incubator-openwhisk/issues/116#issuecomment-336632898 [Composer](https://github.com/ibm-functions/composer/) provides capabilities for defining parameters that are scoped to individual actions, and also forwarding parameters around sequence segments. https://github.com/ibm-functions/composer. 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] dgrove-oss commented on issue #2828: option to use docker pause/unpause instead of docker-runc pause/unpause
dgrove-oss commented on issue #2828: option to use docker pause/unpause instead of docker-runc pause/unpause URL: https://github.com/apache/incubator-openwhisk/pull/2828#issuecomment-336512882 PG2 2167 - completed successfully 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] dgrove-oss commented on issue #68: Update counchdb image init script to avoid python package installatio?
dgrove-oss commented on issue #68: Update counchdb image init script to avoid python package installatio? URL: https://github.com/apache/incubator-openwhisk-deploy-kube/pull/68#issuecomment-336559299 This is one of the images that has kube-specific changes to it (motivating my email to the dev list a little while ago). Can we augment the jenkins job to also publish kube-specific images? 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] dubeejw commented on a change in pull request #2847: Treat action code as attachments for created/updated actions
dubeejw commented on a change in pull request #2847: Treat action code as attachments for created/updated actions URL: https://github.com/apache/incubator-openwhisk/pull/2847#discussion_r144643287 ## File path: common/scala/src/main/scala/whisk/core/entity/Exec.scala ## @@ -241,21 +241,33 @@ protected[core] object Exec extends ArgNormalizer[Exec] with DefaultJsonProtocol manifest.attached .map { a => - val jar: Attachment[String] = { -// java actions once stored the attachment in "jar" instead of "code" -obj.fields.get("code").orElse(obj.fields.get("jar")) + // java actions once stored the attachment in "jar" instead of "code" + val code = obj.fields.get("code").orElse(obj.fields.get("jar")) + + val binary: Boolean = code match { +case Some(JsString(c)) => isBinaryCode(c) +case _ => + obj.fields.get("binary") match { +case Some(JsBoolean(b)) => b +case _ => false + } + } + + val attachment: Attachment[String] = { +code } map { attFmt[String].read(_) } getOrElse { -throw new DeserializationException( - s"'code' must be a valid base64 string in 'exec' for '$kind' actions") +throw new DeserializationException(s"'code' must be a string defined in 'exec' for '$kind' actions") Review comment: Not sure we will ever get here. Since I think the function in map will work even for an empty string. 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