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: turn on SSL in database edit form show 500 error #16151

Merged
merged 8 commits into from
Aug 10, 2021
Merged

Conversation

hughhhh
Copy link
Member

@hughhhh hughhhh commented Aug 9, 2021

SUMMARY

Getting the following error for editing SSL for DBC UI.

{"errors": [{"message": "'str' object has no attribute 'update'", "error_type": "GENERIC_BACKEND_ERROR", "level": "error", "extra": {"issue_codes": [{"code": 1011, "message": "Issue 1011 - Superset encountered an unexpected error."}]}}]}

To fix this we are going to ast_literal to convert the string object to a dictionary

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@hughhhh hughhhh changed the title fix: error for query.update fix: turn on SSL in database edit form show 500 error Aug 9, 2021
@hughhhh
Copy link
Member Author

hughhhh commented Aug 9, 2021

@AAfghahi

Comment on lines 1415 to 1420
if isinstance(query, str):
# Make sure query isn't an empty string
if not query:
query = {}
else:
query = literal_eval(query)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The proper way to handle this would be fixing the frontend to always send an object, like I commented here: https://github.com/apache/superset/pull/16038/files#r681331894

We need to either fix DatabaseObject.query to have only type object here, or at least fix the FE so that it converts query correctly to an object before doing a request to the backend.

@codecov
Copy link

codecov bot commented Aug 9, 2021

Codecov Report

Merging #16151 (09168b9) into master (f0e3b68) will not change coverage.
The diff coverage is 0.00%.

❗ Current head 09168b9 differs from pull request most recent head 7c74286. Consider uploading reports for the commit 7c74286 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master   #16151   +/-   ##
=======================================
  Coverage   76.62%   76.62%           
=======================================
  Files         996      996           
  Lines       53015    53015           
  Branches     6744     6744           
=======================================
  Hits        40622    40622           
  Misses      12167    12167           
  Partials      226      226           
Flag Coverage Δ
javascript 71.20% <0.00%> (ø)

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

Impacted Files Coverage Δ
...c/views/CRUD/data/database/DatabaseModal/index.tsx 44.15% <0.00%> (ø)

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 f0e3b68...7c74286. Read the comment docs.

@pull-request-size pull-request-size bot added size/S and removed size/XS labels Aug 9, 2021
Copy link
Member

@betodealmeida betodealmeida left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Thanks for fixing this!

@@ -18,6 +18,7 @@
import json
import logging
import re
from ast import literal_eval
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
from ast import literal_eval

Comment on lines 649 to 650
console.log('inValidate', database);
console.log(onCreate);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
console.log('inValidate', database);
console.log(onCreate);

@hughhhh hughhhh merged commit 3f86a54 into master Aug 10, 2021
@rosemarie-chiu
Copy link
Contributor

🏷 2021.31

stevenuray pushed a commit to preset-io/superset that referenced this pull request Aug 11, 2021
* fix error for query.update

* converrt before making request

* fix query params

* remove unchanged files

* this

* update tsconfig

(cherry picked from commit 3f86a54)
@villebro villebro deleted the hugh/fix-ssl-500 branch August 12, 2021 06:36
@villebro villebro added the v1.3 label Aug 12, 2021
villebro pushed a commit that referenced this pull request Aug 16, 2021
* fix error for query.update

* converrt before making request

* fix query params

* remove unchanged files

* this

* update tsconfig

(cherry picked from commit 3f86a54)
opus-42 pushed a commit to opus-42/incubator-superset that referenced this pull request Nov 14, 2021
* fix error for query.update

* converrt before making request

* fix query params

* remove unchanged files

* this

* update tsconfig
cccs-RyanS pushed a commit to CybercentreCanada/superset that referenced this pull request Dec 17, 2021
* fix error for query.update

* converrt before making request

* fix query params

* remove unchanged files

* this

* update tsconfig

(cherry picked from commit 3f86a54)
QAlexBall pushed a commit to QAlexBall/superset that referenced this pull request Dec 28, 2021
* fix error for query.update

* converrt before making request

* fix query params

* remove unchanged files

* this

* update tsconfig
QAlexBall pushed a commit to QAlexBall/superset that referenced this pull request Dec 29, 2021
* fix error for query.update

* converrt before making request

* fix query params

* remove unchanged files

* this

* update tsconfig

(cherry picked from commit 3f86a54)
cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024
* fix error for query.update

* converrt before making request

* fix query params

* remove unchanged files

* this

* update tsconfig

(cherry picked from commit 0f55202)
@mistercrunch mistercrunch added 🍒 1.3.0 🍒 1.3.1 🍒 1.3.2 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.4.0 labels Mar 13, 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:2021.31 size/S v1.3 🍒 1.3.0 🍒 1.3.1 🍒 1.3.2 🚢 1.4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants