Skip to content

fix(query_object): missing series validation not raised an exception#16946

Merged
amitmiran137 merged 1 commit into
apache:masterfrom
nielsen-oss:fix/missing_parts_query_object_validation
Oct 3, 2021
Merged

fix(query_object): missing series validation not raised an exception#16946
amitmiran137 merged 1 commit into
apache:masterfrom
nielsen-oss:fix/missing_parts_query_object_validation

Conversation

@ofekisr
Copy link
Copy Markdown
Contributor

@ofekisr ofekisr commented Oct 3, 2021

Fixing the PR feat: add support for generic series limit

take a look at query_object file, line 280, an exception is not raised when there are missing series

@ofekisr ofekisr force-pushed the fix/missing_parts_query_object_validation branch from 24d0e3a to 581a206 Compare October 3, 2021 10:28
Copy link
Copy Markdown
Member

@amitmiran137 amitmiran137 left a comment

Choose a reason for hiding this comment

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

Looks good to me

@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 3, 2021

Codecov Report

Merging #16946 (24d0e3a) into master (0d0c759) will decrease coverage by 0.07%.
The diff coverage is 57.14%.

❗ Current head 24d0e3a differs from pull request most recent head 581a206. Consider uploading reports for the commit 581a206 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master   #16946      +/-   ##
==========================================
- Coverage   76.98%   76.90%   -0.08%     
==========================================
  Files        1028     1028              
  Lines       55065    55071       +6     
  Branches     7486     7486              
==========================================
- Hits        42390    42351      -39     
- Misses      12427    12472      +45     
  Partials      248      248              
Flag Coverage Δ
hive 81.44% <57.14%> (-0.01%) ⬇️
mysql 81.82% <57.14%> (-0.01%) ⬇️
postgres 81.89% <57.14%> (-0.01%) ⬇️
presto ?
python 82.24% <57.14%> (-0.16%) ⬇️
sqlite 81.50% <57.14%> (-0.01%) ⬇️

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

Impacted Files Coverage Δ
superset/common/query_object.py 92.85% <57.14%> (-1.59%) ⬇️
superset/db_engine_specs/presto.py 84.30% <0.00%> (-5.65%) ⬇️
superset/connectors/sqla/models.py 85.83% <0.00%> (-1.42%) ⬇️
superset/models/core.py 89.26% <0.00%> (-0.74%) ⬇️

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 0d0c759...581a206. Read the comment docs.

@amitmiran137 amitmiran137 merged commit c05c6b3 into apache:master Oct 3, 2021
@villebro
Copy link
Copy Markdown
Member

villebro commented Oct 4, 2021

LGTM, thanks for fixing my oversight @ofekisr ! Good example of the benefits of having good code coverage (I'll try to add a quick test for that)

opus-42 pushed a commit to opus-42/incubator-superset that referenced this pull request Nov 14, 2021
QAlexBall pushed a commit to QAlexBall/superset that referenced this pull request Dec 28, 2021
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.4.0 First shipped in 1.4.0 labels Mar 13, 2024
qfcwell pushed a commit to qfcwell/superset that referenced this pull request May 12, 2026
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 size/M 🚢 1.4.0 First shipped in 1.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants