You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/04/09 05:03:09 UTC
[GitHub] [superset] ktmud opened a new pull request, #19636: chore: skip SIP-68 shadow writing for LTS branches
ktmud opened a new pull request, #19636:
URL: https://github.com/apache/superset/pull/19636
### SUMMARY
Change [SIP-68](https://github.com/apache/superset/pull/17543) db migrations to no-op and stop shadow-writing datasets to new models. We should not release the shadow writing logic to official releases as it increases computation costs yet contributes nothing to end user experience (for now). Given the original migration is expensive and we are likely need to [redo it](#19421) anyway, it's the best to skip it in the LTS branches for now.
This PR should only be cherry-picked into the LTS branches such as 1.5 and 2.0. Once #19421 is ready and the new dataset models started to replace the old models, we'll just re-cut the release from `master` and ignore this PR.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### 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
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] 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
--
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
[GitHub] [superset] villebro commented on pull request #19636: chore: skip SIP-68 shadow writing for LTS branches
Posted by GitBox <gi...@apache.org>.
villebro commented on PR #19636:
URL: https://github.com/apache/superset/pull/19636#issuecomment-1100059514
Removed `v1.5` label to not confuse `cherrytree` when baking rc3.
--
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
[GitHub] [superset] ktmud commented on pull request #19636: chore: skip SIP-68 shadow writing for LTS branches
Posted by GitBox <gi...@apache.org>.
ktmud commented on PR #19636:
URL: https://github.com/apache/superset/pull/19636#issuecomment-1096968880
It seems the Helm Chart has the content as `master`. I'm not familiar with how Helm works and not sure whether I should bump the version in this branch. Maybe leave as it and just cherry-pick without merge?
--
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
[GitHub] [superset] codecov[bot] commented on pull request #19636: chore: skip SIP-68 shadow writing for LTS branches
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #19636:
URL: https://github.com/apache/superset/pull/19636#issuecomment-1093717829
# [Codecov](https://codecov.io/gh/apache/superset/pull/19636?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#19636](https://codecov.io/gh/apache/superset/pull/19636?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (29b5a79) into [1.5](https://codecov.io/gh/apache/superset/commit/77d6207bed81e8bca7e3caaf342eacf10c09eff2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (77d6207) will **increase** coverage by `14.66%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 29b5a79 differs from pull request most recent head 1761616. Consider uploading reports for the commit 1761616 to get more accurate results
```diff
@@ Coverage Diff @@
## 1.5 #19636 +/- ##
===========================================
+ Coverage 51.85% 66.51% +14.66%
===========================================
Files 1686 1686
Lines 65205 65050 -155
Branches 6531 6531
===========================================
+ Hits 33810 43266 +9456
+ Misses 29727 20116 -9611
Partials 1668 1668
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `52.34% <100.00%> (-0.13%)` | :arrow_down: |
| presto | `52.18% <100.00%> (-0.13%)` | :arrow_down: |
| python | `82.12% <100.00%> (+29.39%)` | :arrow_up: |
| sqlite | `81.78% <100.00%> (?)` | |
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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/19636?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/sql\_parse.py](https://codecov.io/gh/apache/superset/pull/19636/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc3FsX3BhcnNlLnB5) | `97.38% <ø> (+34.96%)` | :arrow_up: |
| [superset/common/query\_object\_factory.py](https://codecov.io/gh/apache/superset/pull/19636/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X29iamVjdF9mYWN0b3J5LnB5) | `89.79% <100.00%> (+2.04%)` | :arrow_up: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/19636/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `88.39% <100.00%> (+15.71%)` | :arrow_up: |
| [superset/tables/models.py](https://codecov.io/gh/apache/superset/pull/19636/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdGFibGVzL21vZGVscy5weQ==) | `100.00% <100.00%> (ø)` | |
| [superset/views/base.py](https://codecov.io/gh/apache/superset/pull/19636/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvYmFzZS5weQ==) | `74.83% <100.00%> (+21.85%)` | :arrow_up: |
| [superset/connectors/sqla/utils.py](https://codecov.io/gh/apache/superset/pull/19636/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL3V0aWxzLnB5) | `68.36% <0.00%> (-11.23%)` | :arrow_down: |
| [superset/config.py](https://codecov.io/gh/apache/superset/pull/19636/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `91.95% <0.00%> (+0.30%)` | :arrow_up: |
| [superset/common/utils/query\_cache\_manager.py](https://codecov.io/gh/apache/superset/pull/19636/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29tbW9uL3V0aWxzL3F1ZXJ5X2NhY2hlX21hbmFnZXIucHk=) | `89.41% <0.00%> (+1.17%)` | :arrow_up: |
| [superset/initialization/\_\_init\_\_.py](https://codecov.io/gh/apache/superset/pull/19636/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvaW5pdGlhbGl6YXRpb24vX19pbml0X18ucHk=) | `90.96% <0.00%> (+1.67%)` | :arrow_up: |
| ... and [308 more](https://codecov.io/gh/apache/superset/pull/19636/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/19636?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/19636?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [77d6207...1761616](https://codecov.io/gh/apache/superset/pull/19636?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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
[GitHub] [superset] ktmud commented on pull request #19636: chore: skip SIP-68 shadow writing for LTS branches
Posted by GitBox <gi...@apache.org>.
ktmud commented on PR #19636:
URL: https://github.com/apache/superset/pull/19636#issuecomment-1096923063
> @ktmud this PR appears to have a few linting issues (the underlying branch is now green). We're ready to cut 1.5.0rc3 (there's two fixes in it), but going to wait to get this one in.
Let me fix those
--
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
[GitHub] [superset] ktmud commented on pull request #19636: chore: skip SIP-68 shadow writing for LTS branches
Posted by GitBox <gi...@apache.org>.
ktmud commented on PR #19636:
URL: https://github.com/apache/superset/pull/19636#issuecomment-1093728039
CI failed but the base branch is showing the same error: https://github.com/apache/superset/runs/5886127859?check_suite_focus=true
I'll leave it as is.
--
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
[GitHub] [superset] michael-s-molina commented on pull request #19636: chore: skip SIP-68 shadow writing for LTS branches
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on PR #19636:
URL: https://github.com/apache/superset/pull/19636#issuecomment-1169073638
Should we close this?
--
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
[GitHub] [superset] ktmud closed pull request #19636: chore: skip SIP-68 shadow writing for LTS branches
Posted by GitBox <gi...@apache.org>.
ktmud closed pull request #19636: chore: skip SIP-68 shadow writing for LTS branches
URL: https://github.com/apache/superset/pull/19636
--
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
[GitHub] [superset] michael-s-molina closed pull request #19636: chore: skip SIP-68 shadow writing for LTS branches
Posted by GitBox <gi...@apache.org>.
michael-s-molina closed pull request #19636: chore: skip SIP-68 shadow writing for LTS branches
URL: https://github.com/apache/superset/pull/19636
--
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
[GitHub] [superset] ktmud commented on pull request #19636: chore: skip SIP-68 shadow writing for LTS branches
Posted by GitBox <gi...@apache.org>.
ktmud commented on PR #19636:
URL: https://github.com/apache/superset/pull/19636#issuecomment-1167462124
Given that #19421 is already merged and already significantly improved the migration performance, we can probably skip re-applying this in 2.0 to reduce operational load---otherwise we'd have to make the new migration a no-op and add it back in later releases, too.
--
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
[GitHub] [superset] villebro commented on pull request #19636: chore: skip SIP-68 shadow writing for LTS branches
Posted by GitBox <gi...@apache.org>.
villebro commented on PR #19636:
URL: https://github.com/apache/superset/pull/19636#issuecomment-1094605358
> CI failed but the base branch is showing the same error: https://github.com/apache/superset/runs/5886127859?check_suite_focus=true
>
> I'll leave it as is.
Ugh, I thought I fixed it, but maybe I forgot to commit the fix.
--
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
[GitHub] [superset] michael-s-molina commented on pull request #19636: chore: skip SIP-68 shadow writing for LTS branches
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on PR #19636:
URL: https://github.com/apache/superset/pull/19636#issuecomment-1158826320
@ktmud Can you check CI here? 2.0 is being assembled right now.
--
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
[GitHub] [superset] ktmud commented on pull request #19636: chore: skip SIP-68 shadow writing for LTS branches
Posted by GitBox <gi...@apache.org>.
ktmud commented on PR #19636:
URL: https://github.com/apache/superset/pull/19636#issuecomment-1096929375
@villebro The helm chart failure seems to be related to Helm version updated to a new one in `master`. The base branch didn't trigger an error because the lint job only runs on pull requests. Should we create a new PR to fix it first?
--
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
[GitHub] [superset] villebro commented on pull request #19636: chore: skip SIP-68 shadow writing for LTS branches
Posted by GitBox <gi...@apache.org>.
villebro commented on PR #19636:
URL: https://github.com/apache/superset/pull/19636#issuecomment-1097009540
> It seems the Helm Chart has the content as `master`. I'm not familiar with how Helm works and not sure whether I should bump the version in this branch. Maybe leave as it and just cherry-pick without merge?
This is really confusing. Sounds good, I'll just pick this in as-is 👍
--
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
[GitHub] [superset] villebro commented on pull request #19636: chore: skip SIP-68 shadow writing for LTS branches
Posted by GitBox <gi...@apache.org>.
villebro commented on PR #19636:
URL: https://github.com/apache/superset/pull/19636#issuecomment-1096629411
@ktmud this PR appears to have a few linting issues (the underlying branch is now green). We're ready to cut 1.5.0rc3 (there's two fixes in it), but going to wait to get this one in.
--
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