[GitHub] [incubator-superset] graceguo-supercat edited a comment on issue #8060: SIP-23: Persist SQL Lab state in the backend
graceguo-supercat edited a comment on issue #8060: SIP-23: Persist SQL Lab state in the backend URL: https://github.com/apache/incubator-superset/pull/8060#issuecomment-530622944 Report a few bugs: 1. if you have multiple queries in the same tab, select the 1st query and run, refresh browser, you will lost 2nd query in the tab. But select 2nd query and refresh browser is good.  2. REMOVE_QUERY and REMOVE_QUERY_EDITOR actions seems not working correct. When sql lab is reloaded, removed items in the query history tab are still showing up. Probably need to check if any other actions need to trigger data persist.  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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat edited a comment on issue #8060: SIP-23: Persist SQL Lab state in the backend
graceguo-supercat edited a comment on issue #8060: SIP-23: Persist SQL Lab state in the backend URL: https://github.com/apache/incubator-superset/pull/8060#issuecomment-530622944 Report a few bugs: 1. if you have multiple queries in the same tab, select the 1st query and run, refresh browser, you will lost 2nd query in the tab. But select 2nd query and refresh browser is good.  2. REMOVE_QUERY and REMOVE_QUERY_EDITOR actions seems not working correct. When sql lab is reloaded, removed items in the query history tab are still showing up. Probably need to test all redux state change triggered data persist.  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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] lilila commented on issue #6799: Table view as dashboard filter - don't highlight item or clear filter
lilila commented on issue #6799: Table view as dashboard filter - don't highlight item or clear filter URL: https://github.com/apache/incubator-superset/issues/6799#issuecomment-530721558 I have the same issue with superset 0.34: Filter are not clear clear out. (but highlighting is OK). Any suggestion on 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] lilila edited a comment on issue #6799: Table view as dashboard filter - don't highlight item or clear filter
lilila edited a comment on issue #6799: Table view as dashboard filter - don't highlight item or clear filter URL: https://github.com/apache/incubator-superset/issues/6799#issuecomment-530721558 I have the same issue with superset 0.34: When selected elements are clicked a second time the highlight goes away but the unselected value remains filtered in. Any suggestion on 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] lilila edited a comment on issue #6799: Table view as dashboard filter - don't highlight item or clear filter
lilila edited a comment on issue #6799: Table view as dashboard filter - don't highlight item or clear filter URL: https://github.com/apache/incubator-superset/issues/6799#issuecomment-530721558 I have the same issue with superset 0.34: Unselected elements in table are clear out the highlight goes away but the unselected value remains filtered in. Any suggestion on 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] datinho commented on issue #5845: [READY] [feature] Added dashboard read only with new Rho role
datinho commented on issue #5845: [READY] [feature] Added dashboard read only with new Rho role URL: https://github.com/apache/incubator-superset/pull/5845#issuecomment-530737689 The check fails on TOXENV=cypress-dashboard, it seems a timeout problem (Travis CI side?) that I don't have on my local environment. > 1) Dashboard top-level controls should allow chart level refresh: CypressError: Timed out retrying: cy.wait() timed out waiting 5000ms for the 1st request to the route: 'postJson_8_force'. No request ever occurred. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] codecov-io edited a comment on issue #8163: [SQLLab] Refactor sql json endpoint
codecov-io edited a comment on issue #8163: [SQLLab] Refactor sql json endpoint URL: https://github.com/apache/incubator-superset/pull/8163#issuecomment-529036459 # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=h1) Report > Merging [#8163](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/c566141f251ab8b59449428bf92f21fc20e45858?src=pr&el=desc) will **increase** coverage by `0.01%`. > The diff coverage is `69.49%`. [](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=tree) ```diff @@Coverage Diff @@ ## master#8163 +/- ## == + Coverage 65.95% 65.97% +0.01% == Files 479 479 Lines 2303723054 +17 Branches 2552 2552 == + Hits1519415209 +15 - Misses 7707 7709 +2 Partials 136 136 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/8163/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `70.93% <69.49%> (-0.15%)` | :arrow_down: | | [superset/db\_engine\_specs/postgres.py](https://codecov.io/gh/apache/incubator-superset/pull/8163/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3Bvc3RncmVzLnB5) | `96.55% <0%> (+17.24%)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=footer). Last update [c566141...619b962](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] codecov-io edited a comment on issue #8163: [SQLLab] Refactor sql json endpoint
codecov-io edited a comment on issue #8163: [SQLLab] Refactor sql json endpoint URL: https://github.com/apache/incubator-superset/pull/8163#issuecomment-529036459 # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=h1) Report > Merging [#8163](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/c566141f251ab8b59449428bf92f21fc20e45858?src=pr&el=desc) will **increase** coverage by `0.01%`. > The diff coverage is `69.49%`. [](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=tree) ```diff @@Coverage Diff @@ ## master#8163 +/- ## == + Coverage 65.95% 65.97% +0.01% == Files 479 479 Lines 2303723054 +17 Branches 2552 2552 == + Hits1519415209 +15 - Misses 7707 7709 +2 Partials 136 136 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/8163/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `70.93% <69.49%> (-0.15%)` | :arrow_down: | | [superset/db\_engine\_specs/postgres.py](https://codecov.io/gh/apache/incubator-superset/pull/8163/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3Bvc3RncmVzLnB5) | `96.55% <0%> (+17.24%)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=footer). Last update [c566141...619b962](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] codecov-io edited a comment on issue #8163: [SQLLab] Refactor sql json endpoint
codecov-io edited a comment on issue #8163: [SQLLab] Refactor sql json endpoint URL: https://github.com/apache/incubator-superset/pull/8163#issuecomment-529036459 # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=h1) Report > Merging [#8163](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/c566141f251ab8b59449428bf92f21fc20e45858?src=pr&el=desc) will **increase** coverage by `0.01%`. > The diff coverage is `69.49%`. [](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=tree) ```diff @@Coverage Diff @@ ## master#8163 +/- ## == + Coverage 65.95% 65.97% +0.01% == Files 479 479 Lines 2303723054 +17 Branches 2552 2552 == + Hits1519415209 +15 - Misses 7707 7709 +2 Partials 136 136 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/8163/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `70.93% <69.49%> (-0.15%)` | :arrow_down: | | [superset/db\_engine\_specs/postgres.py](https://codecov.io/gh/apache/incubator-superset/pull/8163/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3Bvc3RncmVzLnB5) | `96.55% <0%> (+17.24%)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=footer). Last update [c566141...619b962](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] codecov-io edited a comment on issue #8163: [SQLLab] Refactor sql json endpoint
codecov-io edited a comment on issue #8163: [SQLLab] Refactor sql json endpoint URL: https://github.com/apache/incubator-superset/pull/8163#issuecomment-529036459 # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=h1) Report > Merging [#8163](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/c566141f251ab8b59449428bf92f21fc20e45858?src=pr&el=desc) will **increase** coverage by `0.01%`. > The diff coverage is `69.49%`. [](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=tree) ```diff @@Coverage Diff @@ ## master#8163 +/- ## == + Coverage 65.95% 65.96% +0.01% == Files 479 479 Lines 2303723053 +16 Branches 2552 2552 == + Hits1519415208 +14 - Misses 7707 7709 +2 Partials 136 136 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/8163/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `70.91% <69.49%> (-0.17%)` | :arrow_down: | | [superset/db\_engine\_specs/postgres.py](https://codecov.io/gh/apache/incubator-superset/pull/8163/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3Bvc3RncmVzLnB5) | `96.55% <0%> (+17.24%)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=footer). Last update [c566141...619b962](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] codecov-io edited a comment on issue #8163: [SQLLab] Refactor sql json endpoint
codecov-io edited a comment on issue #8163: [SQLLab] Refactor sql json endpoint URL: https://github.com/apache/incubator-superset/pull/8163#issuecomment-529036459 # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=h1) Report > Merging [#8163](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/c566141f251ab8b59449428bf92f21fc20e45858?src=pr&el=desc) will **increase** coverage by `0.01%`. > The diff coverage is `69.49%`. [](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=tree) ```diff @@Coverage Diff @@ ## master#8163 +/- ## == + Coverage 65.95% 65.96% +0.01% == Files 479 479 Lines 2303723053 +16 Branches 2552 2552 == + Hits1519415208 +14 - Misses 7707 7709 +2 Partials 136 136 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/8163/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `70.91% <69.49%> (-0.17%)` | :arrow_down: | | [superset/db\_engine\_specs/postgres.py](https://codecov.io/gh/apache/incubator-superset/pull/8163/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3Bvc3RncmVzLnB5) | `96.55% <0%> (+17.24%)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=footer). Last update [c566141...619b962](https://codecov.io/gh/apache/incubator-superset/pull/8163?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] stale[bot] commented on issue #6348: Mapbox dosen't show anything
stale[bot] commented on issue #6348: Mapbox dosen't show anything URL: https://github.com/apache/incubator-superset/issues/6348#issuecomment-530775380 This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue `.pinned` to prevent stale bot from closing the issue. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] kalimuthu123 commented on issue #5269: Filters as a native dashboard (v2) construct
kalimuthu123 commented on issue #5269: Filters as a native dashboard (v2) construct URL: https://github.com/apache/incubator-superset/issues/5269#issuecomment-530856989 the global idea filter request is good we will move forward to nxt levels 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] bkyryliuk opened a new pull request #8219: [WIP] Build better support for schema permissions
bkyryliuk opened a new pull request #8219: [WIP] Build better support for schema permissions URL: https://github.com/apache/incubator-superset/pull/8219 NOTE: WIP on testing on the staging and prod @ dropbox ### CATEGORY Choose one - [X] Bug Fix - [X] Enhancement (new features, refinement) - [ ] Refactor - [ ] Add tests - [ ] Build / Development Environment - [ ] Documentation ### SUMMARY Implementing my old TODO :) This PR implements better support for the schema access permissions, e.g. enables users to use sql lab and view slices, tables in the view models. In order to achieve that schema_perm will be serialized in DB along side with perm and will be updated on the model changes. It will allows us to continue using filters in the model views. Note: moved changes from https://github.com/apache/incubator-superset/pull/8189 to this diff to have more holistic view on the approach. ### TEST PLAN * added unit tests * tested locally [] WIP on testing on the dropbox staging & production instance ### ADDITIONAL INFORMATION - [X] Has associated issue: - [ ] Changes UI - [X] Requires DB Migration. - [X] Confirm DB Migration upgrade and downgrade tested. Note: downgrade doesn't work on sqlite - [ ] Introduces new feature or API - [ ] Removes existing feature or API ### REVIEWERS * @mistercrunch * @dpgaspar * @john-bodley 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] bkyryliuk commented on issue #8189: [Ready] Show databases for users with schema access in sqllab
bkyryliuk commented on issue #8189: [Ready] Show databases for users with schema access in sqllab URL: https://github.com/apache/incubator-superset/pull/8189#issuecomment-530932720 Moved changes to the https://github.com/apache/incubator-superset/pull/8219 closing this RP. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] bkyryliuk closed pull request #8189: [Ready] Show databases for users with schema access in sqllab
bkyryliuk closed pull request #8189: [Ready] Show databases for users with schema access in sqllab URL: https://github.com/apache/incubator-superset/pull/8189 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] betodealmeida commented on issue #8060: SIP-23: Persist SQL Lab state in the backend
betodealmeida commented on issue #8060: SIP-23: Persist SQL Lab state in the backend URL: https://github.com/apache/incubator-superset/pull/8060#issuecomment-530943398 Thanks for testing it, @graceguo-supercat! I'll work on it today! 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] vbassa commented on issue #1952: Add the ability to make annotations on a slice
vbassa commented on issue #1952: Add the ability to make annotations on a slice URL: https://github.com/apache/incubator-superset/issues/1952#issuecomment-530963066 Has this been implemented? Thanks in advance 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] paulvic commented on issue #7739: Custom OAuth issue
paulvic commented on issue #7739: Custom OAuth issue URL: https://github.com/apache/incubator-superset/issues/7739#issuecomment-530988166 I've seen a few comments here suggesting the fix didn't work for them. I don't see them any longer so hopefully they were just config/setup problems. It would great if anyone could give this a comment a 👍 or add just comment if they've confirmed the issue is now resolved. Thanks! 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] paulvic edited a comment on issue #7739: Custom OAuth issue
paulvic edited a comment on issue #7739: Custom OAuth issue URL: https://github.com/apache/incubator-superset/issues/7739#issuecomment-530988166 I've seen a few comments here suggesting the fix didn't work for them. I don't see them any longer so hopefully they were just config/setup problems. It would great if anyone could give this a comment a 👍 or just add a comment if they've confirmed the issue is now resolved. Thanks! 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] robdiciuccio commented on issue #8218: Make pyarrow and msgpack optional
robdiciuccio commented on issue #8218: Make pyarrow and msgpack optional URL: https://github.com/apache/incubator-superset/pull/8218#issuecomment-530998909 Based on the performance gains of msgpack + pyarrow over raw JSON, it would be ideal to standardize on this serialization approach going forward, pending community testing. I'm not familiar with bazel, but can you expand a bit on the difficulties there? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] bkyryliuk commented on issue #8218: [WIP] Make pyarrow and msgpack optional
bkyryliuk commented on issue #8218: [WIP] Make pyarrow and msgpack optional URL: https://github.com/apache/incubator-superset/pull/8218#issuecomment-531002832 @robdiciuccio pyarrow has only binary distributions for the 0.14.1 and https://github.com/apache/arrow/blob/master/python/setup.py#L284 shells out to the cmake during install. There is a workaround via cloning, defining bazel rules and building it internally. I am all in for the standardization, my only preference is to have it optional during the community testing phase. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] bkyryliuk edited a comment on issue #8218: [WIP] Make pyarrow and msgpack optional
bkyryliuk edited a comment on issue #8218: [WIP] Make pyarrow and msgpack optional URL: https://github.com/apache/incubator-superset/pull/8218#issuecomment-531002832 @robdiciuccio pyarrow has only binary distributions for the 0.14.1 and https://github.com/apache/arrow/blob/master/python/setup.py#L284 shells out to the cmake during install. There is a workaround via cloning, defining bazel rules and building it internally. I am all in for the standardization, my only preference is to have it optional during the community testing phase. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] etr2460 commented on a change in pull request #8172: Allow users to estimate query cost before executing it
etr2460 commented on a change in pull request #8172: Allow users to estimate
query cost before executing it
URL:
https://github.com/apache/incubator-superset/pull/8172#discussion_r323965488
##
File path: superset/db_engine_specs/presto.py
##
@@ -373,6 +380,79 @@ def select_star(
presto_cols,
)
+@classmethod
+def estimate_statement_cost(
+cls, statement: str, database, cursor, user_name: str
+) -> Dict[str, str]:
+"""
+Generate a SQL query that estimates the cost of a given statement.
+
+:param statement: A single SQL statement
+:param database: Database instance
+:param cursor: Cursor instance
+:param username: Effective username
+"""
+db_engine_spec = database.db_engine_spec
+parsed_query = ParsedQuery(statement)
+sql = parsed_query.stripped()
+
+SQL_QUERY_MUTATOR = config.get("SQL_QUERY_MUTATOR")
+if SQL_QUERY_MUTATOR:
+sql = SQL_QUERY_MUTATOR(sql, user_name, security_manager, database)
+
+sql = f"EXPLAIN (TYPE IO, FORMAT JSON) {sql}"
+
+db_engine_spec.execute(cursor, sql)
+polled = cursor.poll()
+while polled:
Review comment:
Is this the only way to tell if the query is finished? This seems a little
sketchy, can we not pass a callback or something on success?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] serenajiang opened a new pull request #8220: [wip][sqllab] use celery worker for stop_query
serenajiang opened a new pull request #8220: [wip][sqllab] use celery worker for stop_query URL: https://github.com/apache/incubator-superset/pull/8220 ### CATEGORY Choose one - [ ] Bug Fix - [X] Enhancement (new features, refinement) - [ ] Refactor - [ ] Add tests - [ ] Build / Development Environment - [ ] Documentation ### SUMMARY #8139 introduced retries for `stop_query` because of lock timeout issues. This turned out to be problematic because users now have to wait a few seconds after clicking "stop_query" before receiving a message that stopping the query failed. So, now we run stop_query on a celery worker so that users immediately get a response and don't have to wait for the query to stop. Doesn't fix the fact that stop_query still fails a lot, but hey, baby steps. ### TEST PLAN Checked whether `stop_query` worked as expected locally. Threw some fake exceptions to make sure retries still worked. ### REVIEWERS 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] etr2460 commented on a change in pull request #8172: Allow users to estimate query cost before executing it
etr2460 commented on a change in pull request #8172: Allow users to estimate query cost before executing it URL: https://github.com/apache/incubator-superset/pull/8172#discussion_r323965639 ## File path: superset/models/core.py ## @@ -773,6 +773,13 @@ def name(self): def allows_subquery(self): return self.db_engine_spec.allows_subqueries +@property +def allows_cost_estimate(self): Review comment: add a return type? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] etr2460 commented on a change in pull request #8172: Allow users to estimate query cost before executing it
etr2460 commented on a change in pull request #8172: Allow users to estimate
query cost before executing it
URL:
https://github.com/apache/incubator-superset/pull/8172#discussion_r323965707
##
File path: superset/views/core.py
##
@@ -2391,6 +2396,34 @@ def select_star(self, database_id, table_name,
schema=None):
mydb.select_star(table_name, schema, latest_partition=True,
show_cols=True)
)
+@has_access_api
+@expose("/estimate_query_cost//", methods=["POST"])
+@expose("/estimate_query_cost///", methods=["POST"])
+@event_logger.log_this
+def estimate_query_cost(self, database_id, schema=None):
Review comment:
add types
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] etr2460 commented on a change in pull request #8172: Allow users to estimate query cost before executing it
etr2460 commented on a change in pull request #8172: Allow users to estimate
query cost before executing it
URL:
https://github.com/apache/incubator-superset/pull/8172#discussion_r323965557
##
File path: superset/db_engine_specs/presto.py
##
@@ -373,6 +380,79 @@ def select_star(
presto_cols,
)
+@classmethod
+def estimate_statement_cost(
+cls, statement: str, database, cursor, user_name: str
+) -> Dict[str, str]:
+"""
+Generate a SQL query that estimates the cost of a given statement.
+
+:param statement: A single SQL statement
+:param database: Database instance
+:param cursor: Cursor instance
+:param username: Effective username
+"""
+db_engine_spec = database.db_engine_spec
+parsed_query = ParsedQuery(statement)
+sql = parsed_query.stripped()
+
+SQL_QUERY_MUTATOR = config.get("SQL_QUERY_MUTATOR")
+if SQL_QUERY_MUTATOR:
+sql = SQL_QUERY_MUTATOR(sql, user_name, security_manager, database)
+
+sql = f"EXPLAIN (TYPE IO, FORMAT JSON) {sql}"
+
+db_engine_spec.execute(cursor, sql)
+polled = cursor.poll()
+while polled:
+time.sleep(0.2)
+polled = cursor.poll()
+
+# the output from Presto is a single column and a single row containing
+# JSON:
+#
+# {
+# ...
+# "estimate" : {
+# "outputRowCount" : 8.73265878E8,
+# "outputSizeInBytes" : 3.41425774958E11,
+# "cpuCost" : 3.41425774958E11,
+# "maxMemory" : 0.0,
+# "networkCost" : 3.41425774958E11
+# }
+# }
+data = db_engine_spec.fetch_data(cursor, 1)
+first = data[0][0]
+result = json.loads(first)
+estimate = result["estimate"]
+
+def humanize(value, suffix):
Review comment:
can we add types here?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] etr2460 commented on a change in pull request #8172: Allow users to estimate query cost before executing it
etr2460 commented on a change in pull request #8172: Allow users to estimate query cost before executing it URL: https://github.com/apache/incubator-superset/pull/8172#discussion_r323965788 ## File path: superset/db_engine_specs/base.py ## @@ -148,6 +149,10 @@ class BaseEngineSpec: max_column_name_length = 0 try_remove_schema_from_table_name = True +@classmethod +def get_allow_cost_estimate(cls, version=None): Review comment: add types 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] etr2460 commented on a change in pull request #8172: Allow users to estimate query cost before executing it
etr2460 commented on a change in pull request #8172: Allow users to estimate
query cost before executing it
URL:
https://github.com/apache/incubator-superset/pull/8172#discussion_r323965087
##
File path: superset/db_engine_specs/presto.py
##
@@ -373,6 +380,79 @@ def select_star(
presto_cols,
)
+@classmethod
+def estimate_statement_cost(
+cls, statement: str, database, cursor, user_name: str
+) -> Dict[str, str]:
+"""
+Generate a SQL query that estimates the cost of a given statement.
+
+:param statement: A single SQL statement
+:param database: Database instance
+:param cursor: Cursor instance
+:param username: Effective username
+"""
+db_engine_spec = database.db_engine_spec
+parsed_query = ParsedQuery(statement)
+sql = parsed_query.stripped()
+
+SQL_QUERY_MUTATOR = config.get("SQL_QUERY_MUTATOR")
+if SQL_QUERY_MUTATOR:
+sql = SQL_QUERY_MUTATOR(sql, user_name, security_manager, database)
+
+sql = f"EXPLAIN (TYPE IO, FORMAT JSON) {sql}"
+
+db_engine_spec.execute(cursor, sql)
+polled = cursor.poll()
+while polled:
+time.sleep(0.2)
+polled = cursor.poll()
+
+# the output from Presto is a single column and a single row containing
+# JSON:
+#
+# {
+# ...
+# "estimate" : {
+# "outputRowCount" : 8.73265878E8,
+# "outputSizeInBytes" : 3.41425774958E11,
+# "cpuCost" : 3.41425774958E11,
+# "maxMemory" : 0.0,
+# "networkCost" : 3.41425774958E11
+# }
+# }
+data = db_engine_spec.fetch_data(cursor, 1)
+first = data[0][0]
+result = json.loads(first)
+estimate = result["estimate"]
+
+def humanize(value, suffix):
+if value == "NaN":
+return value
+
+prefixes = ["K", "M", "G", "T", "P", "E", "Z", "Y"]
+prefix = ""
+to_next_prefix = 1000
Review comment:
commenting again, shouldn't this be 1024? And we should make it a const
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] codecov-io commented on issue #8220: [wip][sqllab] use celery worker for stop_query
codecov-io commented on issue #8220: [wip][sqllab] use celery worker for stop_query URL: https://github.com/apache/incubator-superset/pull/8220#issuecomment-531025654 # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8220?src=pr&el=h1) Report > Merging [#8220](https://codecov.io/gh/apache/incubator-superset/pull/8220?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/c566141f251ab8b59449428bf92f21fc20e45858?src=pr&el=desc) will **increase** coverage by `0.01%`. > The diff coverage is `12.5%`. [](https://codecov.io/gh/apache/incubator-superset/pull/8220?src=pr&el=tree) ```diff @@Coverage Diff @@ ## master#8220 +/- ## == + Coverage 65.95% 65.97% +0.01% == Files 479 479 Lines 2303723037 Branches 2552 2552 == + Hits1519415198 +4 + Misses 7707 7703 -4 Partials 136 136 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/8220?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [superset/config.py](https://codecov.io/gh/apache/incubator-superset/pull/8220/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `88.77% <ø> (ø)` | :arrow_up: | | [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/8220/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `71.29% <0%> (+0.21%)` | :arrow_up: | | [superset/sql\_lab.py](https://codecov.io/gh/apache/incubator-superset/pull/8220/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX2xhYi5weQ==) | `75.46% <14.28%> (-2.05%)` | :arrow_down: | | [superset/db\_engine\_specs/postgres.py](https://codecov.io/gh/apache/incubator-superset/pull/8220/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3Bvc3RncmVzLnB5) | `96.55% <0%> (+17.24%)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8220?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8220?src=pr&el=footer). Last update [c566141...ac0029b](https://codecov.io/gh/apache/incubator-superset/pull/8220?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] kristw opened a new pull request #8221: fix: initialize control state for inline control config object
kristw opened a new pull request #8221: fix: initialize control state for inline control config object URL: https://github.com/apache/incubator-superset/pull/8221 ### CATEGORY Choose one - [X] Bug Fix ### SUMMARY The initial state was incorrect when using control config object instead of control name. ### TEST PLAN Tested locally ### REVIEWERS @felixcodes 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] felixcodes commented on issue #8221: fix: initialize control state for inline control config object
felixcodes commented on issue #8221: fix: initialize control state for inline control config object URL: https://github.com/apache/incubator-superset/pull/8221#issuecomment-531029198 LGTM, but can't approve since I'm not an apache member 😂 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] kristw opened a new pull request #8222: refactor: prepare control panel configs for separation into plugins
kristw opened a new pull request #8222: refactor: prepare control panel configs for separation into plugins URL: https://github.com/apache/incubator-superset/pull/8222 ### CATEGORY Choose one - [X] Refactor ### SUMMARY Prepare control panel configs for separation into plugins ### TEST PLAN Tested locally + CI ### REVIEWERS @mistercrunch 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] john-bodley commented on issue #8175: [metric] Exposing security methods for visualizations and query-context
john-bodley commented on issue #8175: [metric] Exposing security methods for visualizations and query-context URL: https://github.com/apache/incubator-superset/pull/8175#issuecomment-531035257 @mistercrunch thanks for deprecating the restricted metrics (as a side note I wonder if there should be another migration to remove all the obsolete `metric_access` permissions). I've updated the PR which removes the now obsolete metric checks. Are you still supportive of having the: - `assert_query_context_permission` - `assert_viz_permission` methods which now have the same behavior as the datasource access but simply provide more context, i.e., the fully defined current context, if a custom security manager would like to override. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] codecov-io commented on issue #8222: refactor: prepare control panel configs for separation into plugins
codecov-io commented on issue #8222: refactor: prepare control panel configs for separation into plugins URL: https://github.com/apache/incubator-superset/pull/8222#issuecomment-531036694 # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8222?src=pr&el=h1) Report > Merging [#8222](https://codecov.io/gh/apache/incubator-superset/pull/8222?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/c566141f251ab8b59449428bf92f21fc20e45858?src=pr&el=desc) will **increase** coverage by `0.02%`. > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/incubator-superset/pull/8222?src=pr&el=tree) ```diff @@Coverage Diff @@ ## master#8222 +/- ## == + Coverage 65.95% 65.97% +0.02% == Files 479 479 Lines 2303723037 Branches 2552 2552 == + Hits1519415199 +5 + Misses 7707 7702 -5 Partials 136 136 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/8222?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [superset/assets/src/explore/controls.jsx](https://codecov.io/gh/apache/incubator-superset/pull/8222/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9leHBsb3JlL2NvbnRyb2xzLmpzeA==) | `41.26% <ø> (ø)` | :arrow_up: | | [superset/db\_engine\_specs/postgres.py](https://codecov.io/gh/apache/incubator-superset/pull/8222/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3Bvc3RncmVzLnB5) | `96.55% <0%> (+17.24%)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8222?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8222?src=pr&el=footer). Last update [c566141...0dc6230](https://codecov.io/gh/apache/incubator-superset/pull/8222?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] codecov-io edited a comment on issue #8222: refactor: prepare control panel configs for separation into plugins
codecov-io edited a comment on issue #8222: refactor: prepare control panel configs for separation into plugins URL: https://github.com/apache/incubator-superset/pull/8222#issuecomment-531036694 # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8222?src=pr&el=h1) Report > Merging [#8222](https://codecov.io/gh/apache/incubator-superset/pull/8222?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/c566141f251ab8b59449428bf92f21fc20e45858?src=pr&el=desc) will **increase** coverage by `0.02%`. > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/incubator-superset/pull/8222?src=pr&el=tree) ```diff @@Coverage Diff @@ ## master#8222 +/- ## == + Coverage 65.95% 65.97% +0.02% == Files 479 479 Lines 2303723037 Branches 2552 2552 == + Hits1519415199 +5 + Misses 7707 7702 -5 Partials 136 136 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/8222?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [superset/assets/src/explore/controls.jsx](https://codecov.io/gh/apache/incubator-superset/pull/8222/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9leHBsb3JlL2NvbnRyb2xzLmpzeA==) | `41.26% <ø> (ø)` | :arrow_up: | | [superset/db\_engine\_specs/postgres.py](https://codecov.io/gh/apache/incubator-superset/pull/8222/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3Bvc3RncmVzLnB5) | `96.55% <0%> (+17.24%)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8222?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8222?src=pr&el=footer). Last update [c566141...0dc6230](https://codecov.io/gh/apache/incubator-superset/pull/8222?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] john-bodley merged pull request #8214: [talisman] Enforcing HTTP for status checks
john-bodley merged pull request #8214: [talisman] Enforcing HTTP for status checks URL: https://github.com/apache/incubator-superset/pull/8214 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] john-bodley edited a comment on issue #8175: [metric] Exposing security methods for visualizations and query-context
john-bodley edited a comment on issue #8175: [metric] Exposing security methods for visualizations and query-context URL: https://github.com/apache/incubator-superset/pull/8175#issuecomment-531035257 @mistercrunch thanks for deprecating the restricted metrics (as a side note I wonder if there should be another migration to remove all the obsolete `metric_access` permissions). I've updated the PR which removes the now obsolete metric checks. Are you still supportive of having the: - `assert_query_context_permission` - `assert_viz_permission` methods which now have the same behavior as the datasource access but simply provide more context, i.e., the fully defined current context, if a custom security manager would like to override. Let me know how best to proceed. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] betodealmeida commented on a change in pull request #8060: SIP-23: Persist SQL Lab state in the backend
betodealmeida commented on a change in pull request #8060: SIP-23: Persist SQL
Lab state in the backend
URL:
https://github.com/apache/incubator-superset/pull/8060#discussion_r324000859
##
File path: superset/assets/src/SqlLab/components/TabbedSqlEditors.jsx
##
@@ -68,6 +69,20 @@ class TabbedSqlEditors extends React.PureComponent {
this.removeAllOtherQueryEditors =
this.removeAllOtherQueryEditors.bind(this);
}
componentDidMount() {
+// migrate query editor and associated tables state to server
Review comment:
@graceguo-supercat, when a user first opens SQL Lab after enabling the
feature, we'll try to migrate their state to the backend. Each query editor
(and each table schema and each query) we'll be sent to the backend, and if the
result is successful it's removed from the local storage.
These are multiple requests. If a request fails, it simply isn't deleted
from local storage, and the next time they reload SQL Lab that failed state
will attempt to be migrated again. For a while, a user might have mixed state,
with some query editors living in the backend and some living in the local
storage. This shouldn't happen, but if it does, things will be fine.
We have to run the migration every time the component is mounted because we
don't know when the user's state will be already migrated, and this will happen
at different times for different users — maybe months after the feature is
enabled, since it happens the first time they open SQL Lab.
Keep in mind that this code is a no-op if they user's state has already been
migrated. In that case, the array defined by
```javascript
this.props.queryEditors.filter(qe => qe.inLocalStorage)
```
is empty, and nothing happens here.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] zhaoyongjie opened a new pull request #8223: Fix sync failed when datasource is schema-less
zhaoyongjie opened a new pull request #8223: Fix sync failed when datasource is schema-less URL: https://github.com/apache/incubator-superset/pull/8223 ### CATEGORY Choose one - [x] Bug Fix - [ ] Enhancement (new features, refinement) - [ ] Refactor - [ ] Add tests - [ ] Build / Development Environment - [ ] Documentation ### SUMMARY ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF ### TEST PLAN ### 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 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] stale[bot] closed issue #7364: Firefox Error while fetching Database list
stale[bot] closed issue #7364: Firefox Error while fetching Database list URL: https://github.com/apache/incubator-superset/issues/7364 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] betodealmeida commented on issue #8060: SIP-23: Persist SQL Lab state in the backend
betodealmeida commented on issue #8060: SIP-23: Persist SQL Lab state in the backend URL: https://github.com/apache/incubator-superset/pull/8060#issuecomment-531098189 @graceguo-supercat, thanks for testing this thoroughly, I had completely missed those two cases. I fixed them, and did another look at all the actions — everything seems to be covered 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] betodealmeida commented on issue #8213: Better distinction between tables and views, and show CREATE VIEW
betodealmeida commented on issue #8213: Better distinction between tables and views, and show CREATE VIEW URL: https://github.com/apache/incubator-superset/pull/8213#issuecomment-531098278 @mistercrunch is this good to go as soon as tests pass? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] betodealmeida commented on a change in pull request #8172: Allow users to estimate query cost before executing it
betodealmeida commented on a change in pull request #8172: Allow users to
estimate query cost before executing it
URL:
https://github.com/apache/incubator-superset/pull/8172#discussion_r324036255
##
File path: superset/db_engine_specs/presto.py
##
@@ -373,6 +380,79 @@ def select_star(
presto_cols,
)
+@classmethod
+def estimate_statement_cost(
+cls, statement: str, database, cursor, user_name: str
+) -> Dict[str, str]:
+"""
+Generate a SQL query that estimates the cost of a given statement.
+
+:param statement: A single SQL statement
+:param database: Database instance
+:param cursor: Cursor instance
+:param username: Effective username
+"""
+db_engine_spec = database.db_engine_spec
+parsed_query = ParsedQuery(statement)
+sql = parsed_query.stripped()
+
+SQL_QUERY_MUTATOR = config.get("SQL_QUERY_MUTATOR")
+if SQL_QUERY_MUTATOR:
+sql = SQL_QUERY_MUTATOR(sql, user_name, security_manager, database)
+
+sql = f"EXPLAIN (TYPE IO, FORMAT JSON) {sql}"
+
+db_engine_spec.execute(cursor, sql)
+polled = cursor.poll()
+while polled:
+time.sleep(0.2)
+polled = cursor.poll()
+
+# the output from Presto is a single column and a single row containing
+# JSON:
+#
+# {
+# ...
+# "estimate" : {
+# "outputRowCount" : 8.73265878E8,
+# "outputSizeInBytes" : 3.41425774958E11,
+# "cpuCost" : 3.41425774958E11,
+# "maxMemory" : 0.0,
+# "networkCost" : 3.41425774958E11
+# }
+# }
+data = db_engine_spec.fetch_data(cursor, 1)
+first = data[0][0]
+result = json.loads(first)
+estimate = result["estimate"]
+
+def humanize(value, suffix):
+if value == "NaN":
+return value
+
+prefixes = ["K", "M", "G", "T", "P", "E", "Z", "Y"]
+prefix = ""
+to_next_prefix = 1000
Review comment:
Sorry, I replied to your comment but I think I resolved the conversation.
This is used not just for bytes, but also for cpu and network cost, so 1000 is
the correct unit. Also, 1000 is the correct unit for the prefixes K, M, G, etc.
For 1024 the prefixes are Ki, Mi, Gi.
Eg, 1024 B = 1 KiB = 1.024 KB.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] betodealmeida commented on a change in pull request #8172: Allow users to estimate query cost before executing it
betodealmeida commented on a change in pull request #8172: Allow users to
estimate query cost before executing it
URL:
https://github.com/apache/incubator-superset/pull/8172#discussion_r324036500
##
File path: superset/db_engine_specs/presto.py
##
@@ -373,6 +380,79 @@ def select_star(
presto_cols,
)
+@classmethod
+def estimate_statement_cost(
+cls, statement: str, database, cursor, user_name: str
+) -> Dict[str, str]:
+"""
+Generate a SQL query that estimates the cost of a given statement.
+
+:param statement: A single SQL statement
+:param database: Database instance
+:param cursor: Cursor instance
+:param username: Effective username
+"""
+db_engine_spec = database.db_engine_spec
+parsed_query = ParsedQuery(statement)
+sql = parsed_query.stripped()
+
+SQL_QUERY_MUTATOR = config.get("SQL_QUERY_MUTATOR")
+if SQL_QUERY_MUTATOR:
+sql = SQL_QUERY_MUTATOR(sql, user_name, security_manager, database)
+
+sql = f"EXPLAIN (TYPE IO, FORMAT JSON) {sql}"
+
+db_engine_spec.execute(cursor, sql)
+polled = cursor.poll()
+while polled:
+time.sleep(0.2)
+polled = cursor.poll()
+
+# the output from Presto is a single column and a single row containing
+# JSON:
+#
+# {
+# ...
+# "estimate" : {
+# "outputRowCount" : 8.73265878E8,
+# "outputSizeInBytes" : 3.41425774958E11,
+# "cpuCost" : 3.41425774958E11,
+# "maxMemory" : 0.0,
+# "networkCost" : 3.41425774958E11
+# }
+# }
+data = db_engine_spec.fetch_data(cursor, 1)
+first = data[0][0]
+result = json.loads(first)
+estimate = result["estimate"]
+
+def humanize(value, suffix):
Review comment:
+1
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] betodealmeida commented on a change in pull request #8172: Allow users to estimate query cost before executing it
betodealmeida commented on a change in pull request #8172: Allow users to estimate query cost before executing it URL: https://github.com/apache/incubator-superset/pull/8172#discussion_r324038995 ## File path: superset/models/core.py ## @@ -773,6 +773,13 @@ def name(self): def allows_subquery(self): return self.db_engine_spec.allows_subqueries +@property +def allows_cost_estimate(self): Review comment: +1 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] betodealmeida commented on a change in pull request #8172: Allow users to estimate query cost before executing it
betodealmeida commented on a change in pull request #8172: Allow users to
estimate query cost before executing it
URL:
https://github.com/apache/incubator-superset/pull/8172#discussion_r324039248
##
File path: superset/views/core.py
##
@@ -2391,6 +2396,34 @@ def select_star(self, database_id, table_name,
schema=None):
mydb.select_star(table_name, schema, latest_partition=True,
show_cols=True)
)
+@has_access_api
+@expose("/estimate_query_cost//", methods=["POST"])
+@expose("/estimate_query_cost///", methods=["POST"])
+@event_logger.log_this
+def estimate_query_cost(self, database_id, schema=None):
Review comment:
+1
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] betodealmeida commented on a change in pull request #8172: Allow users to estimate query cost before executing it
betodealmeida commented on a change in pull request #8172: Allow users to estimate query cost before executing it URL: https://github.com/apache/incubator-superset/pull/8172#discussion_r324039462 ## File path: superset/db_engine_specs/base.py ## @@ -148,6 +149,10 @@ class BaseEngineSpec: max_column_name_length = 0 try_remove_schema_from_table_name = True +@classmethod +def get_allow_cost_estimate(cls, version=None): Review comment: +1 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] betodealmeida commented on a change in pull request #8172: Allow users to estimate query cost before executing it
betodealmeida commented on a change in pull request #8172: Allow users to
estimate query cost before executing it
URL:
https://github.com/apache/incubator-superset/pull/8172#discussion_r324041463
##
File path: superset/db_engine_specs/presto.py
##
@@ -373,6 +380,79 @@ def select_star(
presto_cols,
)
+@classmethod
+def estimate_statement_cost(
+cls, statement: str, database, cursor, user_name: str
+) -> Dict[str, str]:
+"""
+Generate a SQL query that estimates the cost of a given statement.
+
+:param statement: A single SQL statement
+:param database: Database instance
+:param cursor: Cursor instance
+:param username: Effective username
+"""
+db_engine_spec = database.db_engine_spec
+parsed_query = ParsedQuery(statement)
+sql = parsed_query.stripped()
+
+SQL_QUERY_MUTATOR = config.get("SQL_QUERY_MUTATOR")
+if SQL_QUERY_MUTATOR:
+sql = SQL_QUERY_MUTATOR(sql, user_name, security_manager, database)
+
+sql = f"EXPLAIN (TYPE IO, FORMAT JSON) {sql}"
+
+db_engine_spec.execute(cursor, sql)
+polled = cursor.poll()
+while polled:
Review comment:
Yeah, let me simplify 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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] betodealmeida commented on issue #8172: Allow users to estimate query cost before executing it
betodealmeida commented on issue #8172: Allow users to estimate query cost before executing it URL: https://github.com/apache/incubator-superset/pull/8172#issuecomment-531105306 @etr2460, I added types and cleaned up the query execution. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] villebro commented on a change in pull request #8172: Allow users to estimate query cost before executing it
villebro commented on a change in pull request #8172: Allow users to estimate
query cost before executing it
URL:
https://github.com/apache/incubator-superset/pull/8172#discussion_r324043052
##
File path: superset/db_engine_specs/presto.py
##
@@ -373,6 +380,79 @@ def select_star(
presto_cols,
)
+@classmethod
+def estimate_statement_cost(
+cls, statement: str, database, cursor, user_name: str
+) -> Dict[str, str]:
+"""
+Generate a SQL query that estimates the cost of a given statement.
+
+:param statement: A single SQL statement
+:param database: Database instance
+:param cursor: Cursor instance
+:param username: Effective username
+"""
+db_engine_spec = database.db_engine_spec
+parsed_query = ParsedQuery(statement)
+sql = parsed_query.stripped()
+
+SQL_QUERY_MUTATOR = config.get("SQL_QUERY_MUTATOR")
+if SQL_QUERY_MUTATOR:
+sql = SQL_QUERY_MUTATOR(sql, user_name, security_manager, database)
+
+sql = f"EXPLAIN (TYPE IO, FORMAT JSON) {sql}"
+
+db_engine_spec.execute(cursor, sql)
+polled = cursor.poll()
+while polled:
+time.sleep(0.2)
+polled = cursor.poll()
+
+# the output from Presto is a single column and a single row containing
+# JSON:
+#
+# {
+# ...
+# "estimate" : {
+# "outputRowCount" : 8.73265878E8,
+# "outputSizeInBytes" : 3.41425774958E11,
+# "cpuCost" : 3.41425774958E11,
+# "maxMemory" : 0.0,
+# "networkCost" : 3.41425774958E11
+# }
+# }
+data = db_engine_spec.fetch_data(cursor, 1)
+first = data[0][0]
+result = json.loads(first)
+estimate = result["estimate"]
+
+def humanize(value, suffix):
+if value == "NaN":
+return value
+
+prefixes = ["K", "M", "G", "T", "P", "E", "Z", "Y"]
+prefix = ""
+to_next_prefix = 1000
Review comment:
I had the same concern as @etr2460 , learned something new here (had to
google to double check) 👍
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] villebro commented on issue #8213: Better distinction between tables and views, and show CREATE VIEW
villebro commented on issue #8213: Better distinction between tables and views, and show CREATE VIEW URL: https://github.com/apache/incubator-superset/pull/8213#issuecomment-531108760 LGTM. It hit me, that we should also offer a "get CREATE TABLE statement". I often like to add comments for both table and columns, and those would be convenient to see with the click of a button. So down the line perhaps `get_create_view` could be refactored to `get_create_statement` and would return the "CREATE TABLE" or "CREATE VIEW", depending on the type. But let's save that for a later PR. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] betodealmeida commented on issue #8060: SIP-23: Persist SQL Lab state in the backend
betodealmeida commented on issue #8060: SIP-23: Persist SQL Lab state in the backend URL: https://github.com/apache/incubator-superset/pull/8060#issuecomment-531115915 @graceguo-supercat, note that if you want to test this you need to downgrade the DB and upgrade again, since I modified the model. Before, I was storing the SQL as a reference to a `query` object, but then I hit the bug you found with selected text. Now, the SQL is stored explicitly with the state of the tab. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
