You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "kgabryje (via GitHub)" <gi...@apache.org> on 2024/03/01 09:59:00 UTC
[PR] fix: Migration for single metric in Big Number with Time Comparison [superset]
kgabryje opened a new pull request, #27351:
URL: https://github.com/apache/superset/pull/27351
### SUMMARY
Due to changing `metrics` control to `metric` in Big Number with Time Comparison chart, the existing charts were broken due to missing `metric` in form data. This PR adds a migration which takes the first item from `metrics` and applies it to the new `metric` control.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
1. Revert commit fd4f9ac (`feat: Use standardized controls in Big Number with Time Comparison`)
2. Create a Big Number with Time Comparison chart and save it
3. Un-revert the commit
4. Verify that now the chart is broken - no metric is added
5. Run the migration
6. Verify that the chart now works and `metric` control has a value
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [x] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [x] Migration is atomic, supports rollback & is backwards-compatible
- [x] Confirm DB migration upgrade and downgrade tested
- [x] Runtime estimates and downtime expectations provided
Current: 0.13 s
10+: 0.12 s
100+: 0.12 s
1000+: 0.20 s
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Migration for single metric in Big Number with Time Comparison [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on PR #27351:
URL: https://github.com/apache/superset/pull/27351#issuecomment-1974135056
We should move all to `metrics` and delete `metric` forever!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Migration for single metric in Big Number with Time Comparison [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #27351:
URL: https://github.com/apache/superset/pull/27351#issuecomment-1972887105
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/27351?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 59.69%. Comparing base [(`89d49e5`)](https://app.codecov.io/gh/apache/superset/commit/89d49e55bbf6dfa045fb11822ea5760ab28a1362?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`8ef8c5a`)](https://app.codecov.io/gh/apache/superset/pull/27351?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #27351 +/- ##
==========================================
- Coverage 67.37% 59.69% -7.68%
==========================================
Files 1908 1908
Lines 74550 74550
Branches 8317 8317
==========================================
- Hits 50225 44502 -5723
- Misses 22272 27995 +5723
Partials 2053 2053
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/27351/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [mysql](https://app.codecov.io/gh/apache/superset/pull/27351/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/27351/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/27351/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `53.73% <ø> (?)` | |
| [python](https://app.codecov.io/gh/apache/superset/pull/27351/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `62.31% <ø> (-15.93%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/27351/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/27351/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.50% <ø> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/27351?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Migration for single metric in Big Number with Time Comparison [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on PR #27351:
URL: https://github.com/apache/superset/pull/27351#issuecomment-1974135981
Also - apologies - super early in the history of Superset (pre-React!) I created multiple metrics-related control instead of creating a more versatile one. I think back then it was backend generate by wtforms and they had different components for single/multiple select...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Migration for single metric in Big Number with Time Comparison [superset]
Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje merged PR #27351:
URL: https://github.com/apache/superset/pull/27351
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Migration for single metric in Big Number with Time Comparison [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on PR #27351:
URL: https://github.com/apache/superset/pull/27351#issuecomment-1977109837
🏷️ preset:2024.8
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org