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(database): ensure pk_constraint is JSON serializable #13147

Merged
merged 1 commit into from
Feb 16, 2021

Conversation

villebro
Copy link
Member

@villebro villebro commented Feb 16, 2021

SUMMARY

Some databases return sets in the dict payload (they should be lists according to the docs) when calling inspector.get_pk_constraint which doesn't serialize to JSON. In addition, an empty set evaluates to truthy vs lists that evaluate to falsy, causing trouble downstream. By normalizing to JSON serializable datatypes we ensure that the response for the external metadata endpoint will return a valid response.

TEST PLAN

Local testing

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Requires DB Migration.
  • Confirm DB Migration upgrade and downgrade tested.
  • Introduces new feature or API
  • Removes existing feature or API

@codecov-io
Copy link

codecov-io commented Feb 16, 2021

Codecov Report

Merging #13147 (4df135c) into master (2ce7982) will increase coverage by 0.38%.
The diff coverage is 63.63%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #13147      +/-   ##
==========================================
+ Coverage   53.06%   53.44%   +0.38%     
==========================================
  Files         489      494       +5     
  Lines       17314    17382      +68     
  Branches     4482     4512      +30     
==========================================
+ Hits         9187     9290     +103     
+ Misses       8127     8092      -35     
Flag Coverage Δ
cypress 53.44% <63.63%> (+0.38%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...end/src/SqlLab/components/RunQueryActionButton.tsx 52.77% <ø> (ø)
superset-frontend/src/chart/ChartContainer.jsx 100.00% <ø> (ø)
superset-frontend/src/chart/ChartRenderer.jsx 75.67% <0.00%> (-1.04%) ⬇️
...perset-frontend/src/common/components/Dropdown.tsx 50.00% <ø> (ø)
...rontend/src/components/ListView/CardSortSelect.tsx 78.94% <ø> (ø)
...ontend/src/components/ListViewCard/ImageLoader.tsx 75.00% <0.00%> (ø)
...set-frontend/src/components/URLShortLinkButton.jsx 100.00% <ø> (ø)
...rset-frontend/src/components/URLShortLinkModal.tsx 77.77% <ø> (ø)
...src/dashboard/components/FiltersBadge/selectors.ts 90.00% <ø> (ø)
...src/dashboard/components/HeaderActionsDropdown.jsx 68.49% <ø> (+0.92%) ⬆️
... and 70 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d8c32b8...4df135c. Read the comment docs.

@villebro villebro merged commit 5ab613d into apache:master Feb 16, 2021
@villebro villebro deleted the villebro/set-to-list branch February 16, 2021 10:32
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.2.0 labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels preset-io size/XS 🚢 1.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants