Skip to content

Conversation

@mofojed
Copy link
Member

@mofojed mofojed commented Jul 19, 2021

Fixes #862

Client can now start a session like:

const connection = new IdeConnection({ websocketUrl: 'wss://localhost:10000/api' });
const consoleTypes = await connection.getConsoleTypes();
const session = await connection.startSession(consoleTypes[0]);

Fixes deephaven#862

Client can now start a session like:
```
const connection = new IdeConnection({ websocketUrl: 'wss://localhost:10000/api' });
const consoleTypes = await connection.getConsoleTypes();
const session = await connection.startSession(consoleTypes[0]);
```
Copy link
Member

@niloc132 niloc132 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ConsoleAddress should be explicitly marked as a jstype (though not that instance of the class, let's make a new one for this purpose?), otherwise the compiler can change what names it expects that field to have, and if we were to remove any case where we instantiate it from gwt/java, this code path would stop working entirely.

Also, can you update the examples to use this pattern instead? It seems to make a lot more sense in general to work this way.

And didn't we discuss putting this in IdeClient or some other "worker" level type, rather than making it part of how you connect to the actual ide session? this would make app mode easier, by letting scopes potentially be different.

- Fix up IdeConnection constructor - now it takes two params, one for the URL, then another for connection options
- Change up IdeClient to use new connection options
- Delete the unused classes ConsoleAddress and JsWorkspaceData
- Update some of the examples with new usage. For the examples that have code snippets written in Groovy, I didn't change anything
@mofojed mofojed requested a review from niloc132 July 21, 2021 20:15
Copy link
Member

@niloc132 niloc132 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this update got console.html and simple.html, but the index.html lists console.html, table_viewport.html, table_filter.html, table_sort.html (and format, but that doesnt use server). This doesn't include simple, which is too simple to matter for most people, but glad to have that fixed too.

@mofojed
Copy link
Member Author

mofojed commented Jul 26, 2021

I added another ticket for converting the examples from Groovy (#904)

@mofojed mofojed requested a review from niloc132 July 26, 2021 15:10
@mofojed mofojed requested a review from niloc132 July 26, 2021 18:37
Tested with the following code snippets. Ensured the auth token was read/decoded properly when it was passed in.

// Options unset
var c = new dh.IdeConnection('http://localhost:10000')

// Options object set, no auth token set
var options = new dh.IdeConnectionOptions();
var c = new dh.IdeConnection('http://localhost:10000', options);

// Options object set, auth token set
var options = new dh.IdeConnectionOptions();
options.authToken = '1234';
var c = new dh.IdeConnection('http://localhost:10000', options);

// Property map passed in with auth token set
var c = new dh.IdeConnection('http://localhost:10000', { authToken: '1234' });
@mofojed mofojed merged commit 12c7b55 into deephaven:main Jul 27, 2021
@mofojed mofojed deleted the 862-console-types branch July 27, 2021 01:06
jmao-denver pushed a commit to jmao-denver/deephaven-core that referenced this pull request Apr 11, 2022
Fixes deephaven#862

Client can now start a session like:
```
const connection = new IdeConnection( 'http://localhost:10000');
const consoleTypes = await connection.getConsoleTypes();
const session = await connection.startSession(consoleTypes[0]);
```

* Clean up based on Colin's review

- Fix up IdeConnection constructor - now it takes two params, one for the URL, then another for connection options
- Change up IdeClient to use new connection options
- Delete the unused classes ConsoleAddress and JsWorkspaceData
- Update some of the examples with new usage. For the examples that have code snippets written in Groovy, I didn't change anything

Tested with the following code snippets. Ensured the auth token was read/decoded properly when it was passed in.

// Options unset
var c = new dh.IdeConnection('http://localhost:10000')

// Options object set, no auth token set
var options = new dh.IdeConnectionOptions();
var c = new dh.IdeConnection('http://localhost:10000', options);

// Options object set, auth token set
var options = new dh.IdeConnectionOptions();
options.authToken = '1234';
var c = new dh.IdeConnection('http://localhost:10000', options);

// Property map passed in with auth token set
var c = new dh.IdeConnection('http://localhost:10000', { authToken: '1234' });
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expose console types to client

2 participants