Implement websocket/send action #484

jberstler · 2016-05-20T18:19:05Z

This creates a new package called websocket

Implementation is in Node.JS which required adding the ws package to the Node.JS runtime (see #523).
Added unit tests which confirm that a message can be sent to a running WebSocket, as well as handle various error cases.

sjfink · 2016-05-20T18:44:29Z

tests/src/packages/websocket/WebSocketUtils.java

+
+    public TestWebSocketServer(InetSocketAddress address) {
+        super(address);
+        // TODO Auto-generated constructor stub


Remove all TODO comments ... replace with appropriate comments if needed?

rabbah · 2016-05-20T21:00:44Z

core/nodejsAction/Dockerfile

@@ -52,6 +52,7 @@ swagger-tools@0.8.7 \
 tmp@0.0.28 \
 watson-developer-cloud@1.4.1 \
 when@3.7.3 \
+ws@1.1.0 \


will require updating reference.md and generating pr to bluemix docs @csantanapr.

@bjustin-ibm go ahead and add the new npm package in both places

docs/reference.md#runtime-environment

openwhisk/openwhisk_reference.md#runtime-environment

jberstler · 2016-05-31T15:48:21Z

@rabbah The code is ready and based on the history of the catalog folder you seem like the right person to assign this to. The only question I have is whether or not this one action warrants an entirely new package.

rabbah · 2016-05-31T18:13:59Z

Thanks @bjustin-ibm - now regretting using bash for installing packages ;)

rabbah · 2016-05-31T18:16:18Z

catalog/websocket/sendWebSocketMessageAction.js

+  var WebSocket = require('ws');
+  var ws = new WebSocket(uri);
+
+  ws.on('open', function() {


should this exploit a warm action? hmm...

csantanapr · 2016-05-31T18:22:07Z

@rabbah about

Thanks @bjustin-ibm - now regretting using bash for installing packages ;)

Should we create an issue in the backlog for switching to a cross-platform scripting languages (i.e. nodejs)?

rabbah · 2016-05-31T18:23:08Z

tests/src/packages/websocket/WebSocketTests.scala

+            val run = wsk.action.invoke(websocketSendAction, Map("uri" -> badURI.toString.toJson, "payload" -> "This is the message to send".toJson))
+            withActivation(wsk.activation, run) {
+                activation =>
+                    activation.fields("response").asJsObject.fields("success").toString should be("false")


the success property is invariably of type boolean hence should check against false.toJson instead (same above line 81).

rabbah · 2016-05-31T18:31:09Z

personal preferences are for Python and to see all bash scripts replaced some day.

jberstler · 2016-06-15T18:20:14Z

IPGA 64

@rabbah I believe this is ready to merge.

rabbah · 2016-06-15T19:21:05Z

catalog/installWebSocket.sh

+    -a description "Utilities for communicating with WebSockets"
+
+waitForAll
+


@starpit please review annotations below.

field-level descriptions, e.g. what does the uri or payload field mean? "description": "the uri blah..."

i would guess that we should mark the uri field as bindTime? which means that the package-level metadata needs a "parameters" annotation [ { "key": "parameters": value: [{ "name": "uri", bindTime: true, required: true } }]

we don't have an established schema for this yet, but the package also needs these annotations

service icon

branding text e.g. "Cloudant", unicode rather than graphical, but encodes whatever preferred capitalization the provider uses, service marks, etc.

branding icon (optional, e.g. if the service provider has a "Cloudant[TM]" wide-form branding icon)

link to service provider's home page

link to service instantiation broker

re: separability concerns, yes. we can do 1-2, and the 3 as two separate tasks, but we have to make sure that we don't keep kicking the 3 can down the road.

rabbah · 2016-06-15T21:33:59Z

Are the concerns under separable? That requires systematically addressing all the packages since none have these. Whereas 1-2 strike me as in scope.

sjfink · 2016-06-16T14:52:04Z

catalog/installWebSocket.sh

@@ -0,0 +1,29 @@
+#!/bin/bash
+#
+# use the command line interface to install Weather.com package.


Weather.com ? must be typo

Indeed a copy-and-paste-o. Thanks for catching it.

jberstler · 2016-06-16T15:27:39Z

catalog/installWebSocket.sh

+
+createPackage websocket \
+    -a description "Utilities for communicating with WebSockets"
+    -a parameters '[ {"name":"uri", "required":true, "bindTime":true} ]'


@starpit Please verify this is what you were asking for.

yes, @bjustin-ibm awesome, thanks!!

jberstler · 2016-06-17T18:33:34Z

Ok @rabbah... for reals this time 😏 I think this is ready.

rabbah · 2016-06-17T23:33:21Z

catalog/websocket/sendWebSocketMessageAction.js

+        ws.send(payload, function(error) {
+            if (error) {
+                console.log("Error received communicating with websocket: " + error);
+                whisk.error(error)


ws.close() on error? (i can add this as i merge if you confirm.)

This creates a new package called websocket Implementation is in Node.JS and required added the ws package to the Node.JS runtime. Added unit tests which confirm that a message can be sent to a WebSocket running on Bluemix, as well as handle various error cases.

starpit · 2016-06-20T17:28:09Z

@bjustin-ibm hiya, do the parameters have any sensitive information? e.g. does the uri contain passwords?

jberstler · 2016-06-20T17:50:30Z

does the uri contain passwords

@starpit Like any action parameters, they contain exactly what the caller puts in them, and so it is not generally predictable whether or not they will contain sensitive information. In terms of websockets, I don't believe it is necessarily true that the URI will include authentication information, though I suppose it could.

starpit · 2016-06-20T18:04:23Z

ok cool, thanks for the great summary! does a websocket have any sort of lifecycle, e.g. account registration, credentialing, ...?

jberstler · 2016-06-20T19:00:38Z

does a websocket have any sort of lifecycle, e.g. account registration, credentialing, ...?

@starpit websocket is only a communication protocol for transferring arbitrary data, and is not specific to any particular application or service. This OW action is only meant to act as a client to send data to a running websocket server, and will not act as a websocket server.

To answer your question more directly, things like account registration or credentialing could only be accomplished with how a websocket server interprets the data sent to it by clients, and is not something this OW action would actively facilitate or be responsible for.

starpit · 2016-06-20T20:17:56Z

ok thanks @bjustin-ibm

as much as possible, i think we'd like to avoid the "hey this is tantalizing and sounds cool, but to use it you'll need to do stuff we don't tell you about".

e.g. mobile developer wants a websocket server, see this... after all, whisk is trying to help people avoid setting up servers!

maybe we can belay this reaction by naming the package or action ... somehow?

jberstler · 2016-06-20T20:26:05Z

catalog/installWebSocket.sh

+echo Installing WebSocket package.
+
+createPackage websocket \
+    -a description "Utilities for communicating with WebSockets"


@starpit The "websocket" package only has one action named "send". I think the description of the send action (below) makes it pretty clear that it is intended for talking to a websocket server (as opposed to acting as one). The only potentially ambiguous text I could find is the description on the line above. I intentionally left it generic to allow us to add more actions in the future, but if you have a suggestion for making this more clear, we can certainly consider that.

Apart from that, I can't see how naming the package anything but "websocket" could be an improvement, as this term accurately and succinctly describes the specific kind of communication facilitated by the package.

Another option would be to make this "send" action part of some other package (which? I have no idea) and rename the action to something like "websocketSend". That being said, I believe the option of adding this to an existing package was already discussed in #393.

rabbah · 2016-06-20T21:20:58Z

Rebased and merged as 89f2b01.

jberstler changed the title ~~Implement websocket/send action~~ WIP: Implement websocket/send action May 20, 2016

jberstler mentioned this pull request May 20, 2016

websocket package #393

Closed

sjfink reviewed May 20, 2016
View reviewed changes

jberstler force-pushed the websocket-node branch 4 times, most recently from a658722 to e758049 Compare May 20, 2016 19:19

rabbah reviewed May 20, 2016
View reviewed changes

jberstler force-pushed the websocket-node branch 4 times, most recently from e69758f to 06a6ad3 Compare May 31, 2016 15:41

jberstler assigned rabbah May 31, 2016

jberstler force-pushed the websocket-node branch 2 times, most recently from ee9e025 to d965375 Compare May 31, 2016 16:05

jberstler changed the title ~~WIP: Implement websocket/send action~~ Implement websocket/send action May 31, 2016

rabbah reviewed May 31, 2016
View reviewed changes

jberstler changed the title ~~Implement websocket/send action~~ WIP: Implement websocket/send action May 31, 2016

jberstler force-pushed the websocket-node branch from 6bef5db to 8ce387a Compare May 31, 2016 20:11

jberstler force-pushed the websocket-node branch 5 times, most recently from c4e5cec to d43b4c4 Compare June 8, 2016 14:24

jberstler modified the milestones: MILESTONE 7, MILESTONE 6 Jun 8, 2016

jberstler force-pushed the websocket-node branch from d43b4c4 to 5a2b311 Compare June 15, 2016 14:52

jberstler changed the title ~~WIP: Implement websocket/send action~~ Implement websocket/send action Jun 15, 2016

rabbah reviewed Jun 15, 2016
View reviewed changes

sjfink reviewed Jun 16, 2016
View reviewed changes

jberstler force-pushed the websocket-node branch from 5a2b311 to 9469552 Compare June 16, 2016 15:26

jberstler reviewed Jun 16, 2016
View reviewed changes

rabbah reviewed Jun 17, 2016
View reviewed changes

jberstler force-pushed the websocket-node branch from 9469552 to 78bf8a3 Compare June 20, 2016 14:14

jberstler reviewed Jun 20, 2016
View reviewed changes

rabbah closed this Jun 20, 2016

jberstler deleted the websocket-node branch June 21, 2016 13:25

christ unassigned rabbah May 12, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement websocket/send action #484

Implement websocket/send action #484

jberstler commented May 20, 2016 •

edited

Loading

sjfink May 20, 2016

rabbah May 20, 2016

csantanapr May 20, 2016

jberstler commented May 31, 2016

rabbah commented May 31, 2016

rabbah May 31, 2016

csantanapr commented May 31, 2016

rabbah May 31, 2016

rabbah commented May 31, 2016 via email

jberstler commented Jun 15, 2016

rabbah Jun 15, 2016

starpit Jun 15, 2016 •

edited

Loading

starpit Jun 16, 2016

rabbah commented Jun 15, 2016

sjfink Jun 16, 2016

jberstler Jun 16, 2016

jberstler Jun 16, 2016

starpit Jun 17, 2016

jberstler commented Jun 17, 2016

rabbah Jun 17, 2016

starpit commented Jun 20, 2016

jberstler commented Jun 20, 2016

starpit commented Jun 20, 2016

jberstler commented Jun 20, 2016

starpit commented Jun 20, 2016 •

edited

Loading

jberstler Jun 20, 2016 •

edited

Loading

jberstler Jun 20, 2016

rabbah commented Jun 20, 2016

		-a description "Utilities for communicating with WebSockets"

		waitForAll

Implement websocket/send action #484

Implement websocket/send action #484

Conversation

jberstler commented May 20, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jberstler commented May 31, 2016

rabbah commented May 31, 2016

Choose a reason for hiding this comment

csantanapr commented May 31, 2016

Choose a reason for hiding this comment

rabbah commented May 31, 2016 via email

jberstler commented Jun 15, 2016

Choose a reason for hiding this comment

starpit Jun 15, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rabbah commented Jun 15, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jberstler commented Jun 17, 2016

Choose a reason for hiding this comment

starpit commented Jun 20, 2016

jberstler commented Jun 20, 2016

starpit commented Jun 20, 2016

jberstler commented Jun 20, 2016

starpit commented Jun 20, 2016 • edited Loading

jberstler Jun 20, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rabbah commented Jun 20, 2016

jberstler commented May 20, 2016 •

edited

Loading

starpit Jun 15, 2016 •

edited

Loading

starpit commented Jun 20, 2016 •

edited

Loading

jberstler Jun 20, 2016 •

edited

Loading