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

Fix problem responses, bump ERB #516

Merged
merged 2 commits into from
Nov 9, 2023
Merged

Fix problem responses, bump ERB #516

merged 2 commits into from
Nov 9, 2023

Conversation

bmtcril
Copy link
Contributor

@bmtcril bmtcril commented Nov 8, 2023

ERB would previously (accidentally) send response values that were intended to be JSON lists as repr's of Python objects. In some cases that would render them unable to be parsed downstream (especially in SQL, though we tried). This uses the new ERB and cleans up our implementation to use the JSON lists if present.

@bmtcril bmtcril requested a review from Ian2012 November 8, 2023 17:58
@bmtcril
Copy link
Contributor Author

bmtcril commented Nov 8, 2023

@SoryRawyer this, combined with the upstream fix in ERB, should fix the problem responses issues we were seeing if you'd like to take a look.

refresh_superset_assets.sh Outdated Show resolved Hide resolved
The new version of ERB makes user responses to problems
parsable as JSON, allowing our reports to work correctly.
This PR updates the ERB version and updates downstream models to parse the values correctly.
The requirement may not be versioned this way, for instance it breaks tutor init when using a git hash or branch for testing.
@bmtcril bmtcril merged commit 8c632ac into main Nov 9, 2023
5 checks passed
@bmtcril bmtcril deleted the bmtcril/erb_7_0_1 branch November 9, 2023 14:06
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