Add Cylc Review (from Cylc 7)

[wxtim]

Closes cylc/cylc-flow#5937 and cylc/cylc-flow#3441
Requires cylc/cylc-flow#7068 to work correctly (else cylc review is labelled a dead-end).

Sort of requires cylc/release-actions#136 To run coverage later.

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).
• Applied any dependency changes to both setup.cfg (and conda-environment.yml if present).
• Tests are included (or explain why tests are not needed).

Test working can be demonstrated by adding this to CI.

          pip uninstall cylc-flow -y
          pip install git+https://github.com/wxtim/cylc@feat.cylc-review

https://github.com/cylc/cylc-uiserver/actions/runs/19763672460/job/56631293468

• Changelog entry included if this is a change that can affect users
• Cylc-Doc pull request opened if required at cylc/cylc-doc/pull/XXXX.
• If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

[ChrisPaulBennett]

I just had a look at the macOS failures to see I could offer any help........Ooooof.
I'm starting to think that supporting macOS really isn't worth it....

[wxtim]

Tests will fail until other branches merged - this commit's tests shows that they should pass after those are merged.

[ChrisPaulBennett]

Something is not right with the sorting of `last active time`. It appears to be backwards

[oliver-sanders]

@wxtim, are the cylc/release-actions changes required for this in place?

If so, lets run CI with the new config to make sure it's working.

[wxtim]

@wxtim, are the cylc/release-actions changes required for this in place?

If so, lets run CI with the new config to make sure it's working.

cylc/cylc-flow#7232

I hadn't expected cylc/cylc-flow#7068 to be merged, so I was merrily pushing changes to it.

[oliver-sanders]

Merged: cylc/cylc-flow#7232

Kicking tests...

[oliver-sanders]

Tests ran (and passed!), but coverage didn't, likely due to the skip-ci comment!

[wxtim]

https://app.codecov.io/github/cylc/cylc-uiserver/commit/02a662639d8c8ef3b848daaae49a145d7b81a4dc/tree/cylc/uiserver?dropdown=coverage

[wxtim]

@oliver-sanders tests now passing and coverage is covering - it's still low, but it is working.

[oliver-sanders]

The cylc-flow equivalent uses the -a option which sounds reasonable.

[oliver-sanders]

Couple of small things:

• We also need to configure load_roles in order to grant access to the Cylc Review server.
• The jupyter_config.py file we bundle here isn't the best top-level reference material.
• It might be easier to just bundle cylc review as part of the default package (only one dependency anyway).

Here's what I'm thinking:

diff --git a/README.md b/README.md
index a9f3e13a7..360f5ab68 100644
--- a/README.md
+++ b/README.md
@@ -44,6 +44,12 @@ This repository provides the following components of the Cylc system.
   server and handles authentication. It is a
   [JupyterHub](https://github.com/jupyterhub/jupyterhub) server.
 
+* Cylc Review
+
+  A Hub "service" which runs a public web server for viewing user's workflows.
+  It is a Jupyter Hub [JupyterHub](https://github.com/jupyterhub/jupyterhub)
+  service.
+
 
 ## Installation
 
@@ -240,6 +246,23 @@ By default the Cylc part of the UI Server log is written to
 TODO: Link to Jupyter Server logging_config docs when published
 -->
 
+### Review
+
+To enable the Cylc Review web service, register the service using
+`JupyterHub.services` and provide users access to it using
+`JupyterHub.load_roles`:
+
+```python
+from cylc.uiserver.ws import get_review_service_config
+c.JupyterHub.services = [get_review_service_config()]
+c.JupyterHub.load_roles = [
+    {
+        "name": "user",
+        "scopes": ["self", "access:services!cylc-review"],
+    },
+]
+```
+
 ### UI
 
 The UI can be configured via the "Settings" option in the Dashboard.
diff --git a/cylc/uiserver/jupyter_config.py b/cylc/uiserver/jupyter_config.py
index 398a987b9..afde2cb02 100644
--- a/cylc/uiserver/jupyter_config.py
+++ b/cylc/uiserver/jupyter_config.py
@@ -24,7 +24,6 @@ from cylc.uiserver import (
 )
 from cylc.uiserver.app import USER_CONF_ROOT
 from cylc.uiserver.authorise import CylcAuthorizer
-from cylc.uiserver.ws import get_review_service_config
 
 
 # the command the hub should spawn (i.e. the cylc uiserver itself)
@@ -105,8 +104,3 @@ c.CylcUIServer.logging_config = {
 # Lab as a result of being granted the ``access:servers`` permission in Jupyter
 # Hub.
 c.ServerApp.authorizer_class = CylcAuthorizer
-
-
-# Setup Cylc Review - uncomment to start Cylc review with Cylc Hub
-# n.b. if installed with pip optional dependence [review] must be specified.
-# c.JupyterHub.services = [get_review_service_config()]
diff --git a/setup.cfg b/setup.cfg
index 9c6c3f639..53895d491 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -46,11 +46,13 @@ install_requires =
     # don't pin versions, we will get whatever cylc-flow needs, and not the
     # bleeding-edge version.
     cylc-flow==8.7.*
+
     ansimarkup>=1.0.0
+    cherrypy
     jupyter_server>=2.13.0
-    requests
     packaging
     psutil
+    requests
     tornado>=6.5.0
     traitlets>=5.2.1  # required for logging_config (5.2.0 had bugs)
 
@@ -71,7 +73,7 @@ cylc.command =
     gui = cylc.uiserver.scripts.gui:main
     hub = cylc.uiserver.scripts.hub:main [hub]
     hubapp = cylc.uiserver.scripts.hubapp:main [hub]
-    review = cylc.uiserver.scripts.review:main [review]
+    review = cylc.uiserver.scripts.review:main
 
 [options.extras_require]
 hub =
@@ -96,9 +98,6 @@ tests =
     towncrier>=24.7.0
     types-setuptools
     types-requests>2
-review =
-    cherrypy
 all =
     %(hub)s
     %(tests)s
-    %(review)s

[wxtim]

@oliver-sanders - Do your changes work? I can't make them do so!

[oliver-sanders]

???

[oliver-sanders]

Adjust scope to access:services!service=cylc-review

(looks like you have to redefine the scope for the filter?)

[oliver-sanders]

Have got a small problem with workflow sort order:

• Workflows which have not run are considered "newer" than active workflows.
• Workflows with the same "last active time" are sorted in reverse alphabetical order.

[oliver-sanders]

Have got a couple of breakages with the broadcasts functionality...

States:

Events:

[wxtim]

Have got a couple of breakages with the broadcasts functionality...

I believe that I have now fixed this.