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

Change: use redash.utils.json_dumps instead of json.dumps in Python query runner #1419

Merged
merged 1 commit into from
Nov 22, 2016

Conversation

ehfeng
Copy link
Contributor

@ehfeng ehfeng commented Nov 21, 2016

Currently, python runner is returning datetime objects as unicode strings. redash.utils.json_dumps returns them as native python datetime objects.

Currently, python runner is returning datetime objects as unicode strings. `redash.utils.json_dumps` returns them as native python datetime objects.
@ehfeng
Copy link
Contributor Author

ehfeng commented Nov 21, 2016

Still figuring out how to set this up locally, so I made this change blind, based on our gitter discussion.

@arikfr arikfr changed the title Switching to redash.utils.json_dumps Change: use redash.utils.json_dumps instead of json.dumps in Python query runner Nov 22, 2016
@arikfr arikfr merged commit 265c973 into getredash:master Nov 22, 2016
@arikfr
Copy link
Member

arikfr commented Nov 22, 2016

Ping me on the chat if you want a walkthrough of the setup.

@ehfeng
Copy link
Contributor Author

ehfeng commented Nov 22, 2016

@arikfr realized this changes behavior—for hosted redash, it may be worth messaging your users, in case their old queries break.

@arikfr
Copy link
Member

arikfr commented Nov 22, 2016

It's only an extension of the previous behavior - the data types json_dumps handles make the regular json.dumps call raise an exception.

But anyway we don't have the Python query runner enabled in the hosted version, as it's a security risk. While it runs in a sandbox, I quite easily managed to by pass it. And I'm not some hacker...

Thanks for the concern though :)

@ehfeng
Copy link
Contributor Author

ehfeng commented Nov 22, 2016

Dang, lucky. Breaking changes and migrations are always a pain :P

@arikfr
Copy link
Member

arikfr commented Nov 22, 2016

True that 😱

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants