-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make API info available - CLI via Hub/proxy and/or UIS #396
base: master
Are you sure you want to change the base?
Conversation
809ea02
to
6765e1e
Compare
Codecov ReportBase: 80.00% // Head: 79.84% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #396 +/- ##
==========================================
- Coverage 80.00% 79.84% -0.16%
==========================================
Files 12 12
Lines 1160 1181 +21
Branches 197 199 +2
==========================================
+ Hits 928 943 +15
- Misses 195 198 +3
- Partials 37 40 +3
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
854bd9f
to
2f19a0d
Compare
fb2eeb9
to
344246c
Compare
@@ -515,6 +524,22 @@ def set_auth(self): | |||
def initialize_templates(self): | |||
"""Change the jinja templating environment.""" | |||
|
|||
def write_api_info(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can get away without this write.
Apologies if this is stuff you already know, not sure how familiar you are with #370
In ~/.cylc/uiserver/info_files
for every instance there are two files created.. a jpserver-<pid>-open.html
file, which we use for opening the gui with an existing instance (see the code in scripts/gui.py
) and another one, jpserver-<pid>.json
it is this one that contains the same info as the api_info file.
I've compared the files and the info is the same... until you open a new instance using cylc gui --new
, sorry I think that pr has complicated matters a little. At this point, we have one api_info.json
file for two of the jpserver.json files. The api_info file, for me, has recorded a different port. I don't think this is a major problem but I think we can select one of these existing jpserver files and open that in the async request? This should get around the duplication.
The only potential problem I can think of at the moment is in the selection of the file. At the minute, we select a random file for re-using guis. I suspect we may want to select the particular file for the instance that we are interacting with, although I am not 100% sure about this. If so, the file name will be of the format jpserver-<pid>.json
so we only need the process id to get the correct file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I think it is a bit more complicated.
The above scenario was for running the cylc gui
command. For cylc hub
I didn't change the JUPYTER_RUNTIME_DIR
variable, so the files are still generated per instance but they are still saved to the default .local/share/jupyter
directory... see: https://docs.jupyter.org/en/latest/use/jupyter-directories.html#envvar-JUPYTER_RUNTIME_DIR
The info contained looks the same. Not sure what goes on with different users attaching though. Might need some investigation.
Thanks for looking at this @datamel ! Oh that's handy to know, found the hub created one.. a whole pile of them (~600 or so) We should use it, it worked setting
However, a couple of things:
We could possibly just do something like:
(found on some substack) Another thing is, I want to be able to use the hub proxy URL instead of UIS directly (as that may be exposed to a different network).. But I need to be able to scrape the URL from somewhere, and probably add it to this file.. Any ideas? |
Yes, Jupyter do clean them but I think we need to add our own clean logic as it seems patchy (this was discussed recently on element). They don't get cleaned when cylc gui/hub is not shut down cleanly, e.g. killed with a
At the moment we do:
and then randomly select one, rather than using the latest file (note that this file is the html open one, not the json one that you need - they are generated in a pair). I don't fully understand the issue of implementing this comms method - would any uiserver info be suitable, or do we need the particular uiserver that the workflow will be going through? |
Just thinking now that we may want to put them in a separate dir to the one used for
I'll read up about this and get back to you. I'm not fully sure how to do this at the moment! |
The hub located files and the other files look to have identical infomation (not hub specific, tho might pay to check), so I think it might be safe to locate these in the same place. However, for the CLI (and maybe the gui), I would like to have the hub proxy URL available to use (maybe preferentially).. And it would help if we didn't have to read two files (i.e. have the same file contain I would go further to say, instead of starting up a new UIS for the gui, we should reuse existing for gui, no? (regardless of hub started or non-hub) |
Any UIS works for the CLI via UIS, and we can also use the hub proxy url (which may be preferable, as it might be intentionally exposed to a wider network) |
Also, they are not cleaned up when stopping the UIS via the hub interface... |
These changes partially address cylc/cylc-flow#5235
Sibling to cylc/cylc-flow#5267
On spawning the UI Server (Jupyter server), the hub negotiates an API token and puts it as
JUPYTERHUB_API_TOKEN
, along with other variables, into the the environment. If not hub spawned the UI Server will generate one itself.Using the server app's information (
self.serverapp.server_info()
) and overwriting it with HUB info, we can make this available by writing it to a (user read-only) file:(this token changes on each UIS spawn)
This can be used to access the UIS via the UIS url (as above) but also via the hub proxy (
http://localhost:8000/user/sutherlander/
)..And copied (manually only?) to machines on the network for API access there.
Tasks:
Sample:
Requirements check-list
I have read
CONTRIBUTING.md
and added my name as a Code Contributor.Contains logically grouped changes (else tidy your branch by rebase).
Does not contain off-topic changes (use other PRs for other changes).
Appropriate tests are included (test for token use should be included).
Already covered by existing tests (file is written on UIS start).
Appropriate change log entry included.
I have opened a documentation PR at cylc/cylc-doc/pull/XXXX.