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: exception when saving dash #13300

Merged
merged 2 commits into from Feb 24, 2021
Merged

Conversation

amitmiran137
Copy link
Member

@amitmiran137 amitmiran137 commented Feb 23, 2021

SUMMARY

a bug sometimes happens when saving dash

bug.mov

this was the stacktrace from server

Traceback (most recent call last):
  File "/app/superset/views/base.py", line 162, in wraps
    return f(self, *args, **kwargs)
  File "/usr/local/lib/python3.7/site-packages/flask_appbuilder/security/decorators.py", line 151, in wraps
    return f(self, *args, **kwargs)
  File "/app/superset/utils/log.py", line 164, in wrapper
    value = f(*args, **kwargs)
  File "/app/superset/views/core.py", line 1183, in save_dash
    if remote_last_modified_time < current_last_modified_time:

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

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

@amitmiran137 amitmiran137 changed the title fix: project None value fix: exception when saving dash Feb 23, 2021
@codecov-io
Copy link

codecov-io commented Feb 23, 2021

Codecov Report

Merging #13300 (5a4c017) into master (99a0c8a) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #13300   +/-   ##
=======================================
  Coverage   77.15%   77.15%           
=======================================
  Files         866      866           
  Lines       45005    45005           
  Branches     5357     5421   +64     
=======================================
+ Hits        34723    34725    +2     
+ Misses      10159    10157    -2     
  Partials      123      123           
Flag Coverage Δ
cypress 58.51% <ø> (ø)
javascript 62.20% <ø> (ø)
python 80.81% <100.00%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
superset/views/core.py 75.33% <100.00%> (ø)
superset/db_engine_specs/presto.py 88.46% <0.00%> (+0.42%) ⬆️

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 99a0c8a...5a4c017. Read the comment docs.

@junlincc
Copy link
Member

junlincc commented Feb 23, 2021

@amitmiran137 thanks for the PR! 🙏
I have a couple of questions, appreciate if you can educate me.
what user actions trigger these errors usually?

before this change, how user find out the these errors? if you look at the end of the video you'll see the error in a popup with the entire stackstrace storming into the user's face

I tried changing the color schemes a few time in a row and this happend

@junlincc junlincc added the dashboard:error Related to Dashboard errors label Feb 23, 2021
@amitmiran137 amitmiran137 merged commit 29d6420 into master Feb 24, 2021
@amitmiran137 amitmiran137 deleted the chore/handle_save_dash_error branch March 29, 2021 08:24
@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 dashboard:error Related to Dashboard errors size/XS 🚢 1.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants