-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Fix: if you change the result object, python runner wouldn't return any results #503
Conversation
|
||
json_data = json.dumps(self._result) |
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.
@erans FYI.
The problem was here: if you set a different object to result
in the script, then self._result
will no longer reference to 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.
I had to do it due to scoping issue and the support for the print statement
On Wed, Jul 22, 2015, 17:37 Arik Fraimovich notifications@github.com
wrote:
In redash/query_runner/python.py
#503 (comment):
json_data = json.dumps(self._result)
@erans https://github.com/erans FYI.
The problem was here: if you set a different object to result in the
script, then self._result will no longer reference to it.—
Reply to this email directly or view it on GitHub
https://github.com/EverythingMe/redash/pull/503/files#r35219495.
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.
Perhaps I can add a statement to override it.
On Wed, Jul 22, 2015, 17:39 Eran Sandler eran@sandler.co.il wrote:
I had to do it due to scoping issue and the support for the print statement
On Wed, Jul 22, 2015, 17:37 Arik Fraimovich notifications@github.com
wrote:In redash/query_runner/python.py
#503 (comment):
json_data = json.dumps(self._result)
@erans https://github.com/erans FYI.
The problem was here: if you set a different object to result in the
script, then self._result will no longer reference to it.—
Reply to this email directly or view it on GitHub
https://github.com/EverythingMe/redash/pull/503/files#r35219495.
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.
Actually it's better to accumulate the log statements in another variable, and then put them in result["log"], or they might overriden -- I'll do it in this branch.
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.
OK but in that case it may override anything that was set in result["log"]
On Wed, Jul 22, 2015, 17:47 Arik Fraimovich notifications@github.com
wrote:
In redash/query_runner/python.py
#503 (comment):
json_data = json.dumps(self._result)
Actually it's better to accumulate the log statements in another variable,
and then put them in result["log"], or they might overriden -- I'll do it
in this branch.—
Reply to this email directly or view it on GitHub
https://github.com/EverythingMe/redash/pull/503/files#r35220733.
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 could merge if it exists already, but doesn't feel like it worth the hassle. People shouldn't abuse this :)
See fix here: 07b88d0
(removed the need for WeakRef too)
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.
OK. Looks good.
On Wed, Jul 22, 2015, 17:58 Arik Fraimovich notifications@github.com
wrote:
In redash/query_runner/python.py
#503 (comment):
json_data = json.dumps(self._result)
I could merge if it exists already, but doesn't feel like it worth the
hassle. People shouldn't abuse this :)See fix here: 07b88d0
07b88d0
(removed the need for WeakRef too)—
Reply to this email directly or view it on GitHub
https://github.com/EverythingMe/redash/pull/503/files#r35222171.
f955b51
to
21f3346
Compare
Fix: if you change the result object, python runner wouldn't return any results
No description provided.