Skip to content
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

Cylc Review: Handle more characters #6171

Open
wxtim opened this issue Jun 26, 2024 · 3 comments
Open

Cylc Review: Handle more characters #6171

wxtim opened this issue Jun 26, 2024 · 3 comments
Assignees
Labels
bug Something is wrong :(
Milestone

Comments

@wxtim
Copy link
Member

wxtim commented Jun 26, 2024

Currently Cylc Review cannot handle many characters, including those used to create the Cylc logo on the command line. As a result any log file containing the Cylc Logo will be shown as plain text.

Reproducible Example

# flow.cylc
[scheduling]
    [[graph]]
        R1 = foo

[runtime]
    [[foo]]
        # Ensure that Cylc log has examples of unusable chars
        script = cylc message -- "Āńŷ ōf ţħėşę ŵĭļŀ břēąķ ĈŸļç Rëvîéw"

Navigate to Cylc Review (or start it with cylc review start if you don't have a site deployment)

The job activity log for this task is correctly handled by Cylc Review - most of the other log files are displayed as raw text.

@wxtim wxtim added the bug Something is wrong :( label Jun 26, 2024
@wxtim wxtim added this to the 8.4.0 milestone Jun 26, 2024
@ColemanTom
Copy link
Contributor

Is Cylc review back, or does it still need Cylc7/py2 as per #5937 ?

@hjoliver
Copy link
Member

hjoliver commented Jul 8, 2024

It's never gone away. #5937 has recent updates describing current plans. Ultimately we might hope to completely integrate run history into the GUI views, but that's longer term, and it remains to be seen if that would be better than (or preferred over) cylc review style history browsing.

@MetRonnie MetRonnie modified the milestones: 8.4.0, 7.8.x Jul 8, 2024
@oliver-sanders
Copy link
Member

oliver-sanders commented Jul 15, 2024

Sadly, resolving this issue would require a code change (can't just hack the problem away in the environment), this appears to work for me:

diff --git a/lib/cylc/review.py b/lib/cylc/review.py
index 98543dde8..2401a638a 100644
--- a/lib/cylc/review.py
+++ b/lib/cylc/review.py
@@ -596,9 +596,10 @@ class CylcReviewService(object):
                 return cherrypy.lib.static.serve_file(f_name, mime)
             text = open(f_name).read()
         try:
+            text = unicode(text, 'UTF-8')
             if mode in [None, "text"]:
                 text = jinja2.escape(text)
-            lines = [unicode(line) for line in text.splitlines()]
+            lines = text.splitlines()
         except UnicodeDecodeError:
             if path_in_tar:
                 handle.seek(0)

Although this enforces UTF-8, so it likely safer to try this in an except branch, e.g:

try:
    ...
    lines = [unicode(line) for line in text.splitlines()]
    ...
except UnicodeDecodeError:
    try:
        text = unicode(text, 'UTF-8')
        if mode in [None, 'text'];
            text = jinja2.escape(text)
            lines = text.splitlines()
    except UnicodeDecodeError:
        if path_in_tar:
            handle.seek(0)
        ...

To avoid breaking compatibility with any exotic encodings that Cylc Review might currently handle by accident.

@oliver-sanders oliver-sanders self-assigned this Oct 31, 2024
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this issue Oct 31, 2024
* Assume UTF-8.
* Replace any unknown characters to avoid errors.
* Closes cylc#6171
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is wrong :(
Projects
None yet
Development

No branches or pull requests

5 participants