Skip to content

[fix] Updating parse_human_timedelta typing#8436

Merged
john-bodley merged 1 commit intoapache:masterfrom
john-bodley:john-bodley--fix-parse_human_timedelta-typing
Oct 23, 2019
Merged

[fix] Updating parse_human_timedelta typing#8436
john-bodley merged 1 commit intoapache:masterfrom
john-bodley:john-bodley--fix-parse_human_timedelta-typing

Conversation

@john-bodley
Copy link
Member

@john-bodley john-bodley commented Oct 23, 2019

CATEGORY

Choose one

  • Bug Fix
  • Enhancement (new features, refinement)
  • Refactor
  • Add tests
  • Build / Development Environment
  • Documentation

SUMMARY

This PR fixes a regression introduced in #8409 as per the typing for parse_human_timedelta I was led to believe that s needed to be a str yet it was being passed a type of Optional[str]. The logic I added in QueryObject turned out to be wrong as in fact s was of type Optional[str] per: https://github.com/apache/incubator-superset/blob/9fc37ea9f1ff032dea923012613d4e7189ada178/superset/utils/core.py#L303

TEST PLAN

Added additional unit test.

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

REVIEWERS

to: @etr2460 @graceguo-supercat @michellethomas @mistercrunch

@codecov-io
Copy link

codecov-io commented Oct 23, 2019

Codecov Report

Merging #8436 into master will not change coverage.
The diff coverage is 50%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #8436   +/-   ##
=======================================
  Coverage   67.61%   67.61%           
=======================================
  Files         448      448           
  Lines       22486    22486           
  Branches     2364     2364           
=======================================
  Hits        15204    15204           
  Misses       7144     7144           
  Partials      138      138
Impacted Files Coverage Δ
superset/common/query_object.py 50% <0%> (ø) ⬆️
superset/utils/core.py 88.55% <100%> (ø) ⬆️

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 08c6b61...979ff23. Read the comment docs.

@john-bodley john-bodley merged commit 786d770 into apache:master Oct 23, 2019
@john-bodley john-bodley deleted the john-bodley--fix-parse_human_timedelta-typing branch October 23, 2019 23:04
michellethomas pushed a commit to airbnb/superset-fork that referenced this pull request Oct 24, 2019
@mistercrunch mistercrunch added 🍒 0.35.0 Cherry-picked to 0.35.0 🍒 0.35.1 Cherry-picked to 0.35.1 🍒 0.35.2 Cherry-picked to 0.35.2 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.36.0 First shipped in 0.36.0 labels Feb 28, 2024
cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 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 size/XS v0.35 🍒 0.35.0 Cherry-picked to 0.35.0 🍒 0.35.1 Cherry-picked to 0.35.1 🍒 0.35.2 Cherry-picked to 0.35.2 🚢 0.36.0 First shipped in 0.36.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants