fix(dashboard): commit update once #17781
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
Recent changes to dashboard updates caused incorrect partial updates to be committed, resulting in malformed json in some db engines (mysql) and breaking dashboards that contained non ascii characters in headers or custom chart titles. This PR ensures that we only commit when all updates are complete.
This issue emerged in #17570. I verified that #17766 seemed to fix the problem in a test env, but it turns out this problem is likely db specific, so it did not actually address the root cause.
The root cause:
DashboardDAO.update
updates all properties naively, includingjson_metadata
, which includespositions
DashboardDAO.update_charts_owners
commits the changes done inDashboardDAO.update
. Something goes wrong when committing a string with emojis, resulting in a truncateddashboard.json_metadata
for mysql.DashboardDAO.set_dash_metadata
fetchesdashboard.params_dict
, which fails to parse due to malformed json. Error is thrown.Other locations where we save json with emojis inadvertently use
json.dumps(json.loads(...))
to encode the emojis in a mysql-friendly way, so we didn't see this before.TESTING INSTRUCTIONS
Verified change locally with mysql db.
ADDITIONAL INFORMATION
@geido @kgabryje @graceguo-supercat @etr2460