Jupyter server #711
Jupyter server #711
Conversation
oliver-sanders
force-pushed
the
jupyter-server
branch
from
e61ce80 to
3a4ed72
Compare
|
Rebased onto 0.6.0 |
oliver-sanders
force-pushed
the
jupyter-server
branch
from
3a4ed72 to
11e5aa3
Compare
Codecov Report
@@ Coverage Diff @@
## master #711 +/- ##
==========================================
- Coverage 90.41% 90.39% -0.02%
==========================================
Files 83 83
Lines 1575 1583 +8
Branches 105 105
==========================================
+ Hits 1424 1431 +7
- Misses 121 122 +1
Partials 30 30
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
oliver-sanders
force-pushed
the
jupyter-server
branch
from
11e5aa3 to
beee77e
Compare
|
Ready for review @oliver-sanders ? |
oliver-sanders
force-pushed
the
jupyter-server
branch
from
beee77e to
a973097
Compare
|
Sorry, getting behind, yep I think so. |
oliver-sanders
force-pushed
the
jupyter-server
branch
from
2a96259 to
1013dfa
Compare
There was a problem hiding this comment.
Looks good @oliver-sanders ! Couple comments, but no blockers.
The endpoint is now cylc/#/ rather than #/
Just checking, but I think it didn't require any change in Cylc UI since it (VueRouter) is only responsible for the part after /#/? Thanks! 👍
| @@ -46,7 +46,7 @@ describe('GraphQL mixin', () => { | |||
| }) | |||
| const variables = component.vm.variables | |||
| const expected = { | |||
| workflowId: `${user.username}|${workflowName}` | |||
| workflowId: `${user.owner}|${workflowName}` | |||
There was a problem hiding this comment.
With this I think this PR supersedes #724
| this.username = username | ||
| this.groups = groups | ||
| this.created = created | ||
| this.admin = admin | ||
| this.server = server | ||
| this.server = server || '?' // server can be unset |
There was a problem hiding this comment.
Could we just leave it blank? Or is the ? necessary or used elsewhere?
There was a problem hiding this comment.
I think I made this change because I was getting undefined errors from somewhere in the code.
There was a problem hiding this comment.
Quite sure this can be removed @oliver-sanders . Maybe it was the unit test that failed when it was undefined?
From what I recall, I used this server in the past to craft the GraphQL endpoint URL. But then we ran into problems after a user reported s/he couldn't get /extra-path/users/blabla/#/ to work, and then I stopped using server and used the window.location to create the URL.
Only reason the value is still saved, I think, is because it's returned from the JupyterHub REST service, so no harm in leaving it.
There was a problem hiding this comment.
Ah that might have been it, will take a look...
I started this branch a few months back and had to rebase it recently, I think it's smaller now thanks to other UI improvements (like the URL creation thinggy) 👍 so this is probably outdated now. |
* cylc/cylc-uiserver#230 * The UIS is now a Jupyter Server extension. * The endpoint is now `cylc/#/` rather than `#/` * Authentication via token/cookie is now supported. * Sets XSRF header to support new hubless single-user mode.
oliver-sanders
force-pushed
the
jupyter-server
branch
from
1013dfa to
ed64bb4
Compare
|
Rebased and fixed GraphiQL. |
jupyter_server: support Cylc UIS Jupyter Server extension
cylc/#/rather than#/Requirements check-list
CONTRIBUTING.mdand added my name as a Code Contributor.