-
Notifications
You must be signed in to change notification settings - Fork 32
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
Error handling when attempting to check log files #236
Conversation
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.
The proposed change makes full sense. I have been wondering why we didn't encounter it during the Python 2->3 migration as we fixed other diagnostics bugs. I suspect the environment of our CI and production servers, all C7 servers with LANG
and LC_ALL
set to en_US.UTF-8
, sets the correct encoding under the hood.
Tested the bug flagged this PR by creating and importing მიკროსკოპის.fake
so that Unicode characters are in the Blitz log. By default, omero admin diagnostics
completes as expected on our C7 based CI servers. After setting LANG
and LC_ALL
to C
, omero admin diagnostics
fails with the stack trace described in this PR.
With this PR included, the same workflow as described above results in omero admin diagnostics
returning a non-zero status code with both the default and the C locale.
As a side note, there seem to be another error related to the encoding further down the stack
Log dir: /home/omero/workspace/OMERO-server/OMERO.server/var/log exists
Log files: Blitz-0.log 8.6 MB errors=0 warnings=1344
Log files: DropBox.log 1.2 KB
Log files: FileServer.log 114 B
Log files: Indexer-0.log 23.0 KB
Log files: MonitorServer.log 1.1 KB
Log files: PixelData-0.log 5.5 KB
Log files: Processor-0.log 94.8 KB errors=0 warnings=1
Log files: Tables-0.log 20.3 KB errors=0 warnings=3
Log files: TestDropBox.log n/a
Log files: master.err empty
Log files: master.out empty
Log files: Total size 8.78 MB
Error while parsing logs
The following logic likely needs to be updated to set the encoding
omero-py/src/omero/plugins/admin.py
Lines 1400 to 1414 in 46081e1
try: | |
for file in ('Blitz-0.log',): | |
p = self.ctx.dir / "var" / "log" / file | |
import fileinput | |
for line in fileinput.input([str(p)]): | |
lno = fileinput.filelineno() | |
for k, v in list(issues.items()): | |
if k.match(line): | |
self._item('Parsing %s' % file, | |
"[line:%s] %s" % (lno, v)) | |
self.ctx.out("") | |
break | |
except Exception: | |
self.ctx.err("Error while parsing logs") |
Do you want to look into this change or capture and/or handle separately?
Is it possible for non-utf8 characters to end up in |
At least in the context of the omero-py/src/omero/plugins/admin.py Lines 1374 to 1383 in f44d09e
|
@sbesson: In our case |
To summarize today's sleuthing, the consensus was:
There seems to be a separate issue with Java 11 handling unicode somewhere resulting in
|
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.
LGTM. Merging along with #224.
I think this is a Python2 --> Python3 hangover. Python2 would have just silently corrupted incorrect encodings whereas Python3 will unicode everything and try to use what's specified.
UnicodeDecodeError
's can then be thrown duringomero admin diagnostics
if log files contain UTF-8. For example:Default encoding of our vendored
path.py
is specified as ascii here:Since we are using PyPI for nearly everything now might be worth looking into removing our vendored version and adding a dependency on the
path
module./cc @emilroz