-
Notifications
You must be signed in to change notification settings - Fork 598
Conversation
@@ -135,7 +135,7 @@ def read_chunk(filename, offset=-1, length=-1, escape_data=False): | |||
if length == -1: | |||
length = fstat.st_size - offset | |||
|
|||
with open(filename, "r") as fp: |
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.
to be consistent with the previous behaviour (not sure it still makes sense), I think the whole PR change would be something like:
with open(filename, "rb") as fp:
...
if data:
# use permissive decoding and escaping if escape_data is set, otherwise use strict decoding
data = _escape_data(data) if escape_data else data.decode()
....
@@ -135,7 +135,7 @@ def read_chunk(filename, offset=-1, length=-1, escape_data=False): | |||
if length == -1: | |||
length = fstat.st_size - offset | |||
|
|||
with open(filename, "r") as fp: |
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.
to be consistent with the previous behaviour (not sure it still makes sense), I think the whole PR change would be something like:
with open(filename, "rb") as fp:
...
if data:
# use permissive decoding and escaping if escape_data is set, otherwise use strict decoding
data = _escape_data(data) if escape_data else data.decode()
....
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.
it looks like this could be made more compatible with the old behaviour
d09ec1d
to
7747641
Compare
heron/shell/src/python/utils.py
Outdated
return dict(offset=offset, length=len(data), data=data) | ||
|
||
return dict(offset=offset, length=0) | ||
|
||
def _escape_data(data): | ||
return escape(data.decode('utf8', 'replace')) |
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.
once the change to this is reverted, the PR will be compatible with the python2 behaviour - alternativley you could do the decoding in the data = ...
line above
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 code is not working in python3 rather than python compatible. I am trying to modify it because the above error occurs.
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.
with open(filename, "rb") as fp:
If you read it as binary, escape doesn't work.
[E 200723 10:40:39 web:1407] Uncaught exception GET /filedata/./log-files/container_46_pulsar-prod-4_96.log.0?offset=4191685&length=50000 (10.128.139.55)
HTTPServerRequest(protocol='http', host='shared-aurora-044-ladp-jp2p-prod:31516', method='GET', uri='/filedata/./log-files/container_46_pulsar-prod-4_96.log.0?offset=4191685&length=50000', version='HTTP/1.1', remote_ip='10.128.139.55', headers={'Connection': 'close', 'Host': 'shared-aurora-044-ladp-jp2p-prod:31516', 'Accept-Encoding': 'gzip'})
Traceback (most recent call last):
File "/var/lib/mesos/slaves/9a7b96a9-a670-48e3-bedf-051a91ac5a9b-S251/frameworks/c663397e-a472-43bd-92dd-d97027fcf6ce-0000/executors/thermos-www-release-heron-system-access-es-others-46-02c80d1d-6257-4b42-829c-6fafbed25b8d/runs/8597c5be-55d4-4b8b-9e2e-2b8faddc3372/sandbox/.pex/installed_wheels/ba642ca162d5bf8bbf2fad77d02edcf2a3188eb8/tornado-4.0.2-cp36-cp36m-linux_x86_64.whl/tornado/web.py", line 1288, in _stack_context_handle_exception
raise_exc_info((type, value, traceback))
File "<string>", line 3, in raise_exc_info
File "/var/lib/mesos/slaves/9a7b96a9-a670-48e3-bedf-051a91ac5a9b-S251/frameworks/c663397e-a472-43bd-92dd-d97027fcf6ce-0000/executors/thermos-www-release-heron-system-access-es-others-46-02c80d1d-6257-4b42-829c-6fafbed25b8d/runs/8597c5be-55d4-4b8b-9e2e-2b8faddc3372/sandbox/.pex/installed_wheels/ba642ca162d5bf8bbf2fad77d02edcf2a3188eb8/tornado-4.0.2-cp36-cp36m-linux_x86_64.whl/tornado/web.py", line 1475, in wrapper
result = method(self, *args, **kwargs)
File "/var/lib/mesos/slaves/9a7b96a9-a670-48e3-bedf-051a91ac5a9b-S251/frameworks/c663397e-a472-43bd-92dd-d97027fcf6ce-0000/executors/thermos-www-release-heron-system-access-es-others-46-02c80d1d-6257-4b42-829c-6fafbed25b8d/runs/8597c5be-55d4-4b8b-9e2e-2b8faddc3372/sandbox/.pex/code/ca989480a7444b1c46f32b02147ba41568922357/heron/shell/src/python/handlers/filedatahandler.py", line 50, in get
data = utils.read_chunk(path, offset=offset, length=length, escape_data=True)
File "/var/lib/mesos/slaves/9a7b96a9-a670-48e3-bedf-051a91ac5a9b-S251/frameworks/c663397e-a472-43bd-92dd-d97027fcf6ce-0000/executors/thermos-www-release-heron-system-access-es-others-46-02c80d1d-6257-4b42-829c-6fafbed25b8d/runs/8597c5be-55d4-4b8b-9e2e-2b8faddc3372/sandbox/.pex/code/ca989480a7444b1c46f32b02147ba41568922357/heron/shell/src/python/utils.py", line 147, in read_chunk
data = _escape_data(data) if escape_data else data.decode()
File "/var/lib/mesos/slaves/9a7b96a9-a670-48e3-bedf-051a91ac5a9b-S251/frameworks/c663397e-a472-43bd-92dd-d97027fcf6ce-0000/executors/thermos-www-release-heron-system-access-es-others-46-02c80d1d-6257-4b42-829c-6fafbed25b8d/runs/8597c5be-55d4-4b8b-9e2e-2b8faddc3372/sandbox/.pex/code/ca989480a7444b1c46f32b02147ba41568922357/heron/shell/src/python/utils.py", line 153, in _escape_data
return escape(data)
File "/usr/lib64/python3.6/xml/sax/saxutils.py", line 27, in escape
data = data.replace("&", "&")
TypeError: a bytes-like object is required, not 'str'
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.
escape(data.decode())
It works because I modified it.
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.
yep, although I think that decode will still want errors='replace'
to be compatible with the python2 behaviour
1a68b79
to
e15c299
Compare
Signed-off-by: thinker0 <[email protected]>
Signed-off-by: thinker0 <[email protected]>
e15c299
to
5c14c0a
Compare
* Fix log-reader for Python3 Signed-off-by: thinker0 <[email protected]> * typo Signed-off-by: thinker0 <[email protected]> * Revert commit Co-authored-by: thinker0 <[email protected]>
Python3
@Code0x58