Skip to content

fix(examples): calendar chart metric should be metrics#15173

Merged
villebro merged 2 commits into
apache:masterfrom
preset-io:villebro/calendar-metrics
Jun 17, 2021
Merged

fix(examples): calendar chart metric should be metrics#15173
villebro merged 2 commits into
apache:masterfrom
preset-io:villebro/calendar-metrics

Conversation

@villebro
Copy link
Copy Markdown
Member

SUMMARY

One of the example calendar heatmap charts has incorrectly defined the metric control value, while it really should be metrics (the value should also be an array of strings, not a string).

BEFORE

image

AFTER

image

TESTING INSTRUCTIONS

  1. Populate examples data
  2. Open the "Calendar Heatmap" chart
  3. See that previously it didn't work, but after the changes it works properly.

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

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 15, 2021

Codecov Report

Merging #15173 (a88d592) into master (7237324) will decrease coverage by 0.07%.
The diff coverage is n/a.

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

@@            Coverage Diff             @@
##           master   #15173      +/-   ##
==========================================
- Coverage   77.16%   77.09%   -0.08%     
==========================================
  Files         971      971              
  Lines       50243    50243              
  Branches     6112     6112              
==========================================
- Hits        38772    38736      -36     
- Misses      11268    11304      +36     
  Partials      203      203              
Flag Coverage Δ
hive ?
mysql 81.70% <ø> (+<0.01%) ⬆️
postgres 81.71% <ø> (ø)
presto 81.42% <ø> (?)
python 81.95% <ø> (-0.14%) ⬇️
sqlite ?

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

Impacted Files Coverage Δ
superset/examples/random_time_series.py 20.58% <ø> (ø)
superset/db_engines/hive.py 0.00% <0.00%> (-82.15%) ⬇️
superset/db_engine_specs/hive.py 69.20% <0.00%> (-17.21%) ⬇️
superset/db_engine_specs/sqlite.py 91.89% <0.00%> (-5.41%) ⬇️
superset/utils/celery.py 86.20% <0.00%> (-3.45%) ⬇️
superset/result_set.py 96.77% <0.00%> (-1.62%) ⬇️
superset/utils/cache.py 73.26% <0.00%> (-1.00%) ⬇️
superset/utils/core.py 88.93% <0.00%> (-0.13%) ⬇️
superset/models/core.py 90.02% <0.00%> (+0.26%) ⬆️
superset/connectors/sqla/models.py 89.33% <0.00%> (+0.94%) ⬆️
... and 1 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 7237324...e0367fe. Read the comment docs.

@rusackas
Copy link
Copy Markdown
Member

/testenv up

@github-actions
Copy link
Copy Markdown
Contributor

@rusackas Ephemeral environment spinning up at http://54.212.78.129:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

Copy link
Copy Markdown
Member

@rusackas rusackas left a comment

Choose a reason for hiding this comment

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

Not sure why the presto/hive CI action keeps failing, but the code LGTM and it sure seems to have fixed the problem in manual testing!

@rusackas
Copy link
Copy Markdown
Member

I've restarted the presto/hive tests a third time now. If it doesn't pass, it doesn't seem like flakiness. The error doesn't look like it has anything to do with this PR though.

@rusackas
Copy link
Copy Markdown
Member

You might need to rebase this to bring in #15163 to fix the Presto/Hive tests

@villebro villebro force-pushed the villebro/calendar-metrics branch from ed8868b to 7902592 Compare June 17, 2021 08:17
@villebro villebro force-pushed the villebro/calendar-metrics branch from 7902592 to e0367fe Compare June 17, 2021 10:37
@villebro villebro merged commit 1269cc2 into apache:master Jun 17, 2021
@villebro villebro deleted the villebro/calendar-metrics branch June 17, 2021 11:55
@github-actions
Copy link
Copy Markdown
Contributor

Ephemeral environment shutdown and build artifacts deleted.

cccs-RyanS pushed a commit to CybercentreCanada/superset that referenced this pull request Dec 17, 2021
* fix(examples): calendar chart metric should be metrics

* fix presto test
QAlexBall pushed a commit to QAlexBall/superset that referenced this pull request Dec 29, 2021
* fix(examples): calendar chart metric should be metrics

* fix presto test
cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024
* fix(examples): calendar chart metric should be metrics

* fix presto test
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.3.0 First shipped in 1.3.0 labels Mar 12, 2024
qfcwell pushed a commit to qfcwell/superset that referenced this pull request May 12, 2026
* fix(examples): calendar chart metric should be metrics

* fix presto test
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.3.0 First shipped in 1.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants