Skip to content
This repository was archived by the owner on Aug 30, 2022. It is now read-only.

[WIP] Use the new session api #237

Closed
wants to merge 5 commits into from

Conversation

blink1073
Copy link
Contributor

cf #236

cc @minrk

@blink1073
Copy link
Contributor Author

blink1073 commented Oct 6, 2016

The updated integration tests pass (against notebook master, if I comment out the existing session tests that haven't been updated yet).

Copy link
Member

@minrk minrk left a comment

Choose a reason for hiding this comment

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

Looks really nice to me.

if (this.isDisposed) {
return Promise.reject(new Error('Session is disposed'));
}
let data = JSON.stringify({ type });
Copy link
Member

Choose a reason for hiding this comment

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

I did not know that you could use this shorthand for objects!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, available in ES6. I am going to also verify that current master of services still works against notebook master (that the old api still works).

@blink1073
Copy link
Contributor Author

blink1073 commented Oct 7, 2016

Tests pass on services master and notebook master:

Session
[W 09:10:11.490 NotebookApp] Sessions API changed, see updated swagger docs
...
  18 passing (14s)

@minrk
Copy link
Member

minrk commented Oct 10, 2016

Very nice.

@blink1073
Copy link
Contributor Author

Closing as stale.

@blink1073 blink1073 closed this Mar 11, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants