[GitHub] codecov-io commented on issue #4728: [sql_lab]Disabled run query button if sql query editor is empty
codecov-io commented on issue #4728: [sql_lab]Disabled run query button if sql query editor is empty URL: https://github.com/apache/incubator-superset/pull/4728#issuecomment-377861570 # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4728?src=pr=h1) Report > Merging [#4728](https://codecov.io/gh/apache/incubator-superset/pull/4728?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/069d61c53f03608f05a0684572b9a2bb545fd9a6?src=pr=desc) will **not change** coverage. > The diff coverage is `n/a`. [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/4728/graphs/tree.svg?token=KsB0fHcx6l=pr=150=650)](https://codecov.io/gh/apache/incubator-superset/pull/4728?src=pr=tree) ```diff @@ Coverage Diff @@ ## master#4728 +/- ## === Coverage 72.22% 72.22% === Files 204 204 Lines 1532315323 Branches 1180 1180 === Hits1106711067 Misses 4253 4253 Partials33 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/4728?src=pr=tree) | Coverage Δ | | |---|---|---| | [...assets/javascripts/SqlLab/components/SqlEditor.jsx](https://codecov.io/gh/apache/incubator-superset/pull/4728/diff?src=pr=tree#diff-c3VwZXJzZXQvYXNzZXRzL2phdmFzY3JpcHRzL1NxbExhYi9jb21wb25lbnRzL1NxbEVkaXRvci5qc3g=) | `77.88% <ø> (ø)` | :arrow_up: | | [...scripts/SqlLab/components/RunQueryActionButton.jsx](https://codecov.io/gh/apache/incubator-superset/pull/4728/diff?src=pr=tree#diff-c3VwZXJzZXQvYXNzZXRzL2phdmFzY3JpcHRzL1NxbExhYi9jb21wb25lbnRzL1J1blF1ZXJ5QWN0aW9uQnV0dG9uLmpzeA==) | `88% <ø> (ø)` | :arrow_up: | | [...set/assets/javascripts/explore/stores/controls.jsx](https://codecov.io/gh/apache/incubator-superset/pull/4728/diff?src=pr=tree#diff-c3VwZXJzZXQvYXNzZXRzL2phdmFzY3JpcHRzL2V4cGxvcmUvc3RvcmVzL2NvbnRyb2xzLmpzeA==) | `39.25% <0%> (ø)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4728?src=pr=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/4728?src=pr=footer). Last update [069d61c...a337e60](https://codecov.io/gh/apache/incubator-superset/pull/4728?src=pr=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 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
[GitHub] codecov-io commented on issue #4728: [sql_lab]Disabled run query button if sql query editor is empty
codecov-io commented on issue #4728: [sql_lab]Disabled run query button if sql query editor is empty URL: https://github.com/apache/incubator-superset/pull/4728#issuecomment-377861570 # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4728?src=pr=h1) Report > Merging [#4728](https://codecov.io/gh/apache/incubator-superset/pull/4728?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/069d61c53f03608f05a0684572b9a2bb545fd9a6?src=pr=desc) will **not change** coverage. > The diff coverage is `n/a`. [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/4728/graphs/tree.svg?src=pr=KsB0fHcx6l=650=150)](https://codecov.io/gh/apache/incubator-superset/pull/4728?src=pr=tree) ```diff @@ Coverage Diff @@ ## master#4728 +/- ## === Coverage 72.22% 72.22% === Files 204 204 Lines 1532315323 Branches 1180 1180 === Hits1106711067 Misses 4253 4253 Partials33 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/4728?src=pr=tree) | Coverage Δ | | |---|---|---| | [...assets/javascripts/SqlLab/components/SqlEditor.jsx](https://codecov.io/gh/apache/incubator-superset/pull/4728/diff?src=pr=tree#diff-c3VwZXJzZXQvYXNzZXRzL2phdmFzY3JpcHRzL1NxbExhYi9jb21wb25lbnRzL1NxbEVkaXRvci5qc3g=) | `77.88% <ø> (ø)` | :arrow_up: | | [...scripts/SqlLab/components/RunQueryActionButton.jsx](https://codecov.io/gh/apache/incubator-superset/pull/4728/diff?src=pr=tree#diff-c3VwZXJzZXQvYXNzZXRzL2phdmFzY3JpcHRzL1NxbExhYi9jb21wb25lbnRzL1J1blF1ZXJ5QWN0aW9uQnV0dG9uLmpzeA==) | `88% <ø> (ø)` | :arrow_up: | | [...set/assets/javascripts/explore/stores/controls.jsx](https://codecov.io/gh/apache/incubator-superset/pull/4728/diff?src=pr=tree#diff-c3VwZXJzZXQvYXNzZXRzL2phdmFzY3JpcHRzL2V4cGxvcmUvc3RvcmVzL2NvbnRyb2xzLmpzeA==) | `39.25% <0%> (ø)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4728?src=pr=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/4728?src=pr=footer). Last update [069d61c...a337e60](https://codecov.io/gh/apache/incubator-superset/pull/4728?src=pr=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 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
[GitHub] mistercrunch commented on issue #4551: [sql lab] option to disable cross schema search
mistercrunch commented on issue #4551: [sql lab] option to disable cross schema search URL: https://github.com/apache/incubator-superset/pull/4551#issuecomment-377854858 If caching is off it's pretty brutal on the poor database as it hits the endpoint multiple times as users type. A confused user trying to get this to work could results in dozens of stuck calls that scan the metadata tables. On large databases it's not great. Given that, `False` is a better default here, I was just trying to avoid breaking changes. Also if you haven't run that migration, I suggest you schedule downtime for this one. It locked up on our side... This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] mistercrunch commented on issue #4551: [sql lab] option to disable cross schema search
mistercrunch commented on issue #4551: [sql lab] option to disable cross schema search URL: https://github.com/apache/incubator-superset/pull/4551#issuecomment-377854858 If caching is off it's pretty brutal on the poor database as it hits the endpoint multiple times as users type. A confused user trying to get this to work could results in dozens of stuck calls that scan the metadata tables. On large databases it's not great. This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] mistercrunch commented on issue #4706: Build duration/compression analysis
mistercrunch commented on issue #4706: Build duration/compression analysis URL: https://github.com/apache/incubator-superset/issues/4706#issuecomment-377854471 We should totally upgrade, I think it requires a bit of attention beyond just bumping the version though. It's hard to keep up with the pace of node/npm! This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] mistercrunch closed issue #4245: build master branch failed
mistercrunch closed issue #4245: build master branch failed URL: https://github.com/apache/incubator-superset/issues/4245 This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] mistercrunch commented on a change in pull request #4545: add two more number formats, add number formats to table
mistercrunch commented on a change in pull request #4545: add two more number formats, add number formats to table URL: https://github.com/apache/incubator-superset/pull/4545#discussion_r178487022 ## File path: superset/assets/visualizations/table.js ## @@ -90,10 +90,11 @@ function tableVis(slice, payload) { html = `${val}`; } if (isMetric) { -html = slice.d3format(c, val); +const numberFormat = slice.datasource.column_formats && slice.datasource.column_formats[c] || fd.number_format || '.3s'; Review comment: I should have gotten this on the first pass but I think the right thing to do is to add a new optional arg to `slice.d3format` called `fallbackFormat` or something like that. We should also add a comment to that function making it clear what the precedence rule is (column-defined format if available, `fallbackFormat` if it's not, and '0.3s' if no fallback is specified.) This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] mistercrunch commented on issue #3178: Results backend isn't configured
mistercrunch commented on issue #3178: Results backend isn't configured URL: https://github.com/apache/incubator-superset/issues/3178#issuecomment-377852831 Are you sure you `superset_config.py` is in your PYTHONPATH and parsable? This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] mistercrunch commented on issue #3972: [feature] Global customise-able d3 locale
mistercrunch commented on issue #3972: [feature] Global customise-able d3 locale URL: https://github.com/apache/incubator-superset/issues/3972#issuecomment-377852592 FYI there's an easy way to get backend configs to the frontend. Just add the `SUPERSET_D3_LOCALE ` key name here: https://github.com/apache/incubator-superset/blob/8dd052de4bbd8176f78318e3beb81526b1ecddd4/superset/views/base.py#L28 Then it should show up in the redux state on the frontend. This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] mistercrunch commented on issue #3972: [feature] Global customise-able d3 locale
mistercrunch commented on issue #3972: [feature] Global customise-able d3 locale URL: https://github.com/apache/incubator-superset/issues/3972#issuecomment-377852592 FYI there's an easy way to get backend configs to the frontend. Just add the `SUPERSET_D3_LOCALE ` key name here: https://github.com/apache/incubator-superset/blob/8dd052de4bbd8176f78318e3beb81526b1ecddd4/superset/views/base.py#L28 This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] mistercrunch commented on issue #4704: allow customizable caching permissions
mistercrunch commented on issue #4704: allow customizable caching permissions URL: https://github.com/apache/incubator-superset/pull/4704#issuecomment-377852267 Why late-checking deep in the code as opposed to in the view layer? This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] codecov-io commented on issue #4724: Improve database type inference
codecov-io commented on issue #4724: Improve database type inference URL: https://github.com/apache/incubator-superset/pull/4724#issuecomment-377632311 # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4724?src=pr=h1) Report > Merging [#4724](https://codecov.io/gh/apache/incubator-superset/pull/4724?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/069d61c53f03608f05a0684572b9a2bb545fd9a6?src=pr=desc) will **increase** coverage by `0.03%`. > The diff coverage is `100%`. [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/4724/graphs/tree.svg?width=650=pr=KsB0fHcx6l=150)](https://codecov.io/gh/apache/incubator-superset/pull/4724?src=pr=tree) ```diff @@Coverage Diff @@ ## master#4724 +/- ## == + Coverage 72.22% 72.26% +0.03% == Files 204 204 Lines 1532315344 +21 Branches 1180 1180 == + Hits1106711088 +21 Misses 4253 4253 Partials33 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/4724?src=pr=tree) | Coverage Δ | | |---|---|---| | [superset/dataframe.py](https://codecov.io/gh/apache/incubator-superset/pull/4724/diff?src=pr=tree#diff-c3VwZXJzZXQvZGF0YWZyYW1lLnB5) | `99.05% <100%> (+0.3%)` | :arrow_up: | | [superset/viz.py](https://codecov.io/gh/apache/incubator-superset/pull/4724/diff?src=pr=tree#diff-c3VwZXJzZXQvdml6LnB5) | `79.62% <100%> (ø)` | :arrow_up: | | [superset/sql\_lab.py](https://codecov.io/gh/apache/incubator-superset/pull/4724/diff?src=pr=tree#diff-c3VwZXJzZXQvc3FsX2xhYi5weQ==) | `71.97% <100%> (-3.59%)` | :arrow_down: | | [superset/db\_engine\_specs.py](https://codecov.io/gh/apache/incubator-superset/pull/4724/diff?src=pr=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzLnB5) | `53.41% <100%> (+1.51%)` | :arrow_up: | | [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/4724/diff?src=pr=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `86.52% <100%> (ø)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4724?src=pr=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/4724?src=pr=footer). Last update [069d61c...5fd632b](https://codecov.io/gh/apache/incubator-superset/pull/4724?src=pr=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 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
[GitHub] codecov-io commented on issue #4724: Improve database type inference
codecov-io commented on issue #4724: Improve database type inference URL: https://github.com/apache/incubator-superset/pull/4724#issuecomment-377632311 # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4724?src=pr=h1) Report > Merging [#4724](https://codecov.io/gh/apache/incubator-superset/pull/4724?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/069d61c53f03608f05a0684572b9a2bb545fd9a6?src=pr=desc) will **increase** coverage by `0.03%`. > The diff coverage is `100%`. [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/4724/graphs/tree.svg?src=pr=KsB0fHcx6l=650=150)](https://codecov.io/gh/apache/incubator-superset/pull/4724?src=pr=tree) ```diff @@Coverage Diff @@ ## master#4724 +/- ## == + Coverage 72.22% 72.26% +0.03% == Files 204 204 Lines 1532315344 +21 Branches 1180 1180 == + Hits1106711088 +21 Misses 4253 4253 Partials33 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/4724?src=pr=tree) | Coverage Δ | | |---|---|---| | [superset/db\_engine\_specs.py](https://codecov.io/gh/apache/incubator-superset/pull/4724/diff?src=pr=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzLnB5) | `53.41% <100%> (+1.51%)` | :arrow_up: | | [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/4724/diff?src=pr=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `86.52% <100%> (ø)` | :arrow_up: | | [superset/sql\_lab.py](https://codecov.io/gh/apache/incubator-superset/pull/4724/diff?src=pr=tree#diff-c3VwZXJzZXQvc3FsX2xhYi5weQ==) | `71.97% <100%> (-3.59%)` | :arrow_down: | | [superset/dataframe.py](https://codecov.io/gh/apache/incubator-superset/pull/4724/diff?src=pr=tree#diff-c3VwZXJzZXQvZGF0YWZyYW1lLnB5) | `99.05% <100%> (+0.3%)` | :arrow_up: | | [superset/viz.py](https://codecov.io/gh/apache/incubator-superset/pull/4724/diff?src=pr=tree#diff-c3VwZXJzZXQvdml6LnB5) | `79.62% <100%> (ø)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4724?src=pr=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/4724?src=pr=footer). Last update [069d61c...5fd632b](https://codecov.io/gh/apache/incubator-superset/pull/4724?src=pr=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 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
[GitHub] mistercrunch commented on issue #4712: Added localecompare array sort to datahandler, fetchFilterValues
mistercrunch commented on issue #4712: Added localecompare array sort to datahandler, fetchFilterValues URL: https://github.com/apache/incubator-superset/pull/4712#issuecomment-377851383 We should add an `.order` here: https://github.com/apache/incubator-superset/blob/master/superset/connectors/sqla/models.py#L396 and get the database to do the work. This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] mistercrunch commented on issue #4712: Added localecompare array sort to datahandler, fetchFilterValues
mistercrunch commented on issue #4712: Added localecompare array sort to datahandler, fetchFilterValues URL: https://github.com/apache/incubator-superset/pull/4712#issuecomment-377851270 I feel like this should be done on the backend. From my experience it's been sorted, I'm guessing the `SELECT DISTINCT` will often do this, though it's not guaranteed. What backend do you use? This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] ktong commented on issue #4648: Pass timezone to Druid Query granularity
ktong commented on issue #4648: Pass timezone to Druid Query granularity URL: https://github.com/apache/incubator-superset/pull/4648#issuecomment-377850989 Just for your information, this change is matching code at https://github.com/ktong/incubator-superset/blob/00572298c7c6d110aac4789ad95c101b92166802/superset/connectors/druid/models.py#L1020-L1021 This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] mistercrunch closed pull request #4720: Add '.1%' to number format options
mistercrunch closed pull request #4720: Add '.1%' to number format options URL: https://github.com/apache/incubator-superset/pull/4720 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/superset/assets/javascripts/explore/stores/controls.jsx b/superset/assets/javascripts/explore/stores/controls.jsx index f0d00bf030..3b23fb2de7 100644 --- a/superset/assets/javascripts/explore/stores/controls.jsx +++ b/superset/assets/javascripts/explore/stores/controls.jsx @@ -18,6 +18,7 @@ const D3_FORMAT_DOCS = 'D3 format syntax: https://github.com/d3/d3-format'; const D3_FORMAT_OPTIONS = [ ['.1s', '.1s | 12k'], ['.3s', '.3s | 12.3k'], + ['.1%', '.1% | 12.3%'], ['.3%', '.3% | 1234543.210%'], ['.4r', '.4r | 12350'], ['.3f', '.3f | 12345.432'], This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] mistercrunch closed issue #4732: Optimizing Superset rendering
mistercrunch closed issue #4732: Optimizing Superset rendering URL: https://github.com/apache/incubator-superset/issues/4732 This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] codecov-io commented on issue #4724: Improve database type inference
codecov-io commented on issue #4724: Improve database type inference URL: https://github.com/apache/incubator-superset/pull/4724#issuecomment-377632311 # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4724?src=pr=h1) Report > Merging [#4724](https://codecov.io/gh/apache/incubator-superset/pull/4724?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/069d61c53f03608f05a0684572b9a2bb545fd9a6?src=pr=desc) will **increase** coverage by `0.03%`. > The diff coverage is `100%`. [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/4724/graphs/tree.svg?height=150=650=KsB0fHcx6l=pr)](https://codecov.io/gh/apache/incubator-superset/pull/4724?src=pr=tree) ```diff @@Coverage Diff @@ ## master#4724 +/- ## == + Coverage 72.22% 72.26% +0.03% == Files 204 204 Lines 1532315344 +21 Branches 1180 1180 == + Hits1106711088 +21 Misses 4253 4253 Partials33 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/4724?src=pr=tree) | Coverage Δ | | |---|---|---| | [superset/sql\_lab.py](https://codecov.io/gh/apache/incubator-superset/pull/4724/diff?src=pr=tree#diff-c3VwZXJzZXQvc3FsX2xhYi5weQ==) | `71.97% <100%> (-3.59%)` | :arrow_down: | | [superset/db\_engine\_specs.py](https://codecov.io/gh/apache/incubator-superset/pull/4724/diff?src=pr=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzLnB5) | `53.41% <100%> (+1.51%)` | :arrow_up: | | [superset/dataframe.py](https://codecov.io/gh/apache/incubator-superset/pull/4724/diff?src=pr=tree#diff-c3VwZXJzZXQvZGF0YWZyYW1lLnB5) | `99.05% <100%> (+0.3%)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4724?src=pr=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/4724?src=pr=footer). Last update [069d61c...5fd632b](https://codecov.io/gh/apache/incubator-superset/pull/4724?src=pr=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 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
[GitHub] mistercrunch commented on issue #4732: Optimizing Superset rendering
mistercrunch commented on issue #4732: Optimizing Superset rendering URL: https://github.com/apache/incubator-superset/issues/4732#issuecomment-377850328 Duplicate of https://github.com/apache/incubator-superset/issues/4705 This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] mistercrunch commented on a change in pull request #4726: [bugfix] convert metrics to numeric in dataframe
mistercrunch commented on a change in pull request #4726: [bugfix] convert metrics to numeric in dataframe URL: https://github.com/apache/incubator-superset/pull/4726#discussion_r178484892 ## File path: superset/viz.py ## @@ -170,11 +170,21 @@ def get_df(self, query_obj=None): if self.datasource.offset: df[DTTM_ALIAS] += timedelta(hours=self.datasource.offset) df[DTTM_ALIAS] += self.time_shift + +self.df_metrics_to_num(df, query_obj.get('metrics') or []) + df.replace([np.inf, -np.inf], np.nan) fillna = self.get_fillna_for_columns(df.columns) df = df.fillna(fillna) return df +@staticmethod +def df_metrics_to_num(df, metrics): +"""Converting metrics to numeric when pandas.read_sql cannot""" +for col, dtype in df.dtypes.iteritems(): Review comment: That's right. I got confused as I copied the code over and though it must have been backported for compatibility reasons. I'll grep and remove other instances of it where I copied it from. This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] mistercrunch commented on a change in pull request #4726: [bugfix] convert metrics to numeric in dataframe
mistercrunch commented on a change in pull request #4726: [bugfix] convert metrics to numeric in dataframe URL: https://github.com/apache/incubator-superset/pull/4726#discussion_r178484808 ## File path: superset/viz.py ## @@ -170,11 +170,21 @@ def get_df(self, query_obj=None): if self.datasource.offset: df[DTTM_ALIAS] += timedelta(hours=self.datasource.offset) df[DTTM_ALIAS] += self.time_shift + +self.df_metrics_to_num(df, query_obj.get('metrics') or []) Review comment: This way is a bit safer if the key exists with a `None` value it will make it an empty dict. That situation shouldn't occur, but it's a tiny bit safer. This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] mistercrunch commented on a change in pull request #4724: Improve database type inference
mistercrunch commented on a change in pull request #4724: Improve database type inference URL: https://github.com/apache/incubator-superset/pull/4724#discussion_r178483965 ## File path: superset/db_engine_specs.py ## @@ -550,6 +575,11 @@ def adjust_database_uri(cls, uri, selected_schema=None): uri.database = database return uri +@classmethod +def get_datatype(cls, type_code): Review comment: Yes that's the case, I know that Presto should be this method, but I'm unclear on what the base class should do to capture the most common use cases. This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] vibinsv09 opened a new issue #4732: Optimizing Superset rendering
vibinsv09 opened a new issue #4732: Optimizing Superset rendering URL: https://github.com/apache/incubator-superset/issues/4732 Current size of dashboard.entry.js is aroung 7+ MB which causes slight sluggishness in rendering. Are there any optimizations that I can do to reduce its size and improve overall rendering speed? This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] mistercrunch commented on a change in pull request #4724: Improve database type inference
mistercrunch commented on a change in pull request #4724: Improve database type inference URL: https://github.com/apache/incubator-superset/pull/4724#discussion_r178483226 ## File path: superset/db_engine_specs.py ## @@ -495,6 +502,24 @@ def adjust_database_uri(cls, uri, selected_schema=None): uri.database = selected_schema return uri +@classmethod +def get_datatype(cls, type_code): +if not cls.type_code_map: +# only import and store if needed at least once +import MySQLdb +ft = MySQLdb.constants.FIELD_TYPE +cls.type_code_map = { +getattr(ft, k): k +for k in dir(ft) +if not k.startswith('_') +} +print(cls.type_code_map) Review comment: oops This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] mistercrunch commented on a change in pull request #4724: Improve database type inference
mistercrunch commented on a change in pull request #4724: Improve database type inference URL: https://github.com/apache/incubator-superset/pull/4724#discussion_r178483201 ## File path: superset/dataframe.py ## @@ -95,7 +141,7 @@ def agg_func(cls, dtype, column_name): # consider checking for key substring too. if cls.is_id(column_name): return 'count_distinct' -if (issubclass(dtype.type, np.generic) and +if (hasattr(dtype, 'tupe') and issubclass(dtype.type, np.generic) and Review comment: Typo, the intent was to check that `dtype.type` actually exists. I'm not 100% sure whether I actually hit a missing attr or if it was preventative. This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] mistercrunch commented on a change in pull request #4724: Improve database type inference
mistercrunch commented on a change in pull request #4724: Improve database type inference URL: https://github.com/apache/incubator-superset/pull/4724#discussion_r178482757 ## File path: superset/dataframe.py ## @@ -41,26 +63,50 @@ class SupersetDataFrame(object): 'V': None, # raw data (void) } -def __init__(self, df): -self.__df = df.where((pd.notnull(df)), None) +def __init__(self, data, cursor_description, db_engine_spec): +column_names = [] +if cursor_description: +column_names = [col[0] for col in cursor_description] +self.column_names = dedup(column_names) + +# check whether the result set has any nested dict columns +if data: +has_dict_col = any([isinstance(c, dict) for c in data[0]]) +df_data = list(data) if has_dict_col else np.array(data, dtype=object) Review comment: Just relocating the code here. I agree that it's opaque. This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] mistercrunch commented on a change in pull request #4724: Improve database type inference
mistercrunch commented on a change in pull request #4724: Improve database type inference URL: https://github.com/apache/incubator-superset/pull/4724#discussion_r178482757 ## File path: superset/dataframe.py ## @@ -41,26 +63,50 @@ class SupersetDataFrame(object): 'V': None, # raw data (void) } -def __init__(self, df): -self.__df = df.where((pd.notnull(df)), None) +def __init__(self, data, cursor_description, db_engine_spec): +column_names = [] +if cursor_description: +column_names = [col[0] for col in cursor_description] +self.column_names = dedup(column_names) + +# check whether the result set has any nested dict columns +if data: +has_dict_col = any([isinstance(c, dict) for c in data[0]]) +df_data = list(data) if has_dict_col else np.array(data, dtype=object) Review comment: Just relocating the code here. This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] mistercrunch commented on a change in pull request #4724: Improve database type inference
mistercrunch commented on a change in pull request #4724: Improve database type inference URL: https://github.com/apache/incubator-superset/pull/4724#discussion_r178482743 ## File path: superset/dataframe.py ## @@ -41,26 +63,50 @@ class SupersetDataFrame(object): 'V': None, # raw data (void) } -def __init__(self, df): -self.__df = df.where((pd.notnull(df)), None) +def __init__(self, data, cursor_description, db_engine_spec): +column_names = [] +if cursor_description: +column_names = [col[0] for col in cursor_description] +self.column_names = dedup(column_names) + +# check whether the result set has any nested dict columns Review comment: I just moved that code from `sql_lab.convert_results_to_df`, but I think you're right about the comment. I didn't write that line originally. This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] mistercrunch commented on issue #4730: Fix deep equality logic
mistercrunch commented on issue #4730: Fix deep equality logic URL: https://github.com/apache/incubator-superset/pull/4730#issuecomment-377846043 @timifasubaa it's in there, it's just collapsed This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] timifasubaa commented on issue #4730: Fix deep equality logic
timifasubaa commented on issue #4730: Fix deep equality logic URL: https://github.com/apache/incubator-superset/pull/4730#issuecomment-377844251 Since the package.json was updated, can you also update the yarn.lock file? This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] CasperLiu closed issue #4722: How to permit non-Admin role user to see Charts/Dashoards created by Admin role user?
CasperLiu closed issue #4722: How to permit non-Admin role user to see Charts/Dashoards created by Admin role user? URL: https://github.com/apache/incubator-superset/issues/4722 This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] mistercrunch opened a new issue #4731: Missing default on new `MetricsControl`
mistercrunch opened a new issue #4731: Missing default on new `MetricsControl` URL: https://github.com/apache/incubator-superset/issues/4731 Previously we had logic to pick `COUNT(*)` as a default metric so that when clicking on a table or druid datasource it would run a `COUNT(*)` query instead of the current error that says `Empty query?`. If not `count` metric was defined, it would fall back on picking any metric (might not be ideal, but perhaps better than `Empty query?`). It also seems like the new `MetricsControl` does not expose a way to just `count(*)`, maybe we should add this option. @GabeLoins This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] codecov-io commented on issue #4730: Fix deep equality logic
codecov-io commented on issue #4730: Fix deep equality logic URL: https://github.com/apache/incubator-superset/pull/4730#issuecomment-377800983 # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4730?src=pr=h1) Report > Merging [#4730](https://codecov.io/gh/apache/incubator-superset/pull/4730?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/069d61c53f03608f05a0684572b9a2bb545fd9a6?src=pr=desc) will **increase** coverage by `<.01%`. > The diff coverage is `100%`. [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/4730/graphs/tree.svg?src=pr=150=KsB0fHcx6l=650)](https://codecov.io/gh/apache/incubator-superset/pull/4730?src=pr=tree) ```diff @@Coverage Diff @@ ## master#4730 +/- ## == + Coverage 72.22% 72.22% +<.01% == Files 204 204 Lines 1532315324 +1 Branches 1180 1180 == + Hits1106711068 +1 Misses 4253 4253 Partials33 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/4730?src=pr=tree) | Coverage Δ | | |---|---|---| | [superset/assets/javascripts/reduxUtils.js](https://codecov.io/gh/apache/incubator-superset/pull/4730/diff?src=pr=tree#diff-c3VwZXJzZXQvYXNzZXRzL2phdmFzY3JpcHRzL3JlZHV4VXRpbHMuanM=) | `74.57% <100%> (+0.43%)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4730?src=pr=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/4730?src=pr=footer). Last update [069d61c...d9c4671](https://codecov.io/gh/apache/incubator-superset/pull/4730?src=pr=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 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
[GitHub] codecov-io commented on issue #4730: Fix deep equality logic
codecov-io commented on issue #4730: Fix deep equality logic URL: https://github.com/apache/incubator-superset/pull/4730#issuecomment-377800983 # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4730?src=pr=h1) Report > Merging [#4730](https://codecov.io/gh/apache/incubator-superset/pull/4730?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/069d61c53f03608f05a0684572b9a2bb545fd9a6?src=pr=desc) will **increase** coverage by `<.01%`. > The diff coverage is `100%`. [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/4730/graphs/tree.svg?height=150=pr=650=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/4730?src=pr=tree) ```diff @@Coverage Diff @@ ## master#4730 +/- ## == + Coverage 72.22% 72.22% +<.01% == Files 204 204 Lines 1532315324 +1 Branches 1180 1180 == + Hits1106711068 +1 Misses 4253 4253 Partials33 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/4730?src=pr=tree) | Coverage Δ | | |---|---|---| | [superset/assets/javascripts/reduxUtils.js](https://codecov.io/gh/apache/incubator-superset/pull/4730/diff?src=pr=tree#diff-c3VwZXJzZXQvYXNzZXRzL2phdmFzY3JpcHRzL3JlZHV4VXRpbHMuanM=) | `74.57% <100%> (+0.43%)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4730?src=pr=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/4730?src=pr=footer). Last update [069d61c...d9c4671](https://codecov.io/gh/apache/incubator-superset/pull/4730?src=pr=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 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
[GitHub] mistercrunch opened a new pull request #4730: Fix deep equality logic
mistercrunch opened a new pull request #4730: Fix deep equality logic URL: https://github.com/apache/incubator-superset/pull/4730 Zeroed-in on this while a Deck Scatter Plot chart was prompting for refresh on-load. I'm pretty sure using JSON.stringify as a proxy for deep equality is wrong. Not sure how to handle yarn.lock changes here. The changes on yarn.lock are the result of "only" running `yarn add deep-equal` This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] mistercrunch opened a new pull request #4729: [explore] don't prompt to 'Run Query' on viewport change
mistercrunch opened a new pull request #4729: [explore] don't prompt to 'Run Query' on viewport change URL: https://github.com/apache/incubator-superset/pull/4729 This is another take on https://github.com/apache/incubator-superset I couldn't come up with a better name than `dontRefreshOnChange` as I needed a "truthy" attribute to avoid having to touch all other controls. This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] villebro commented on issue #3998: [SQLLab] Allow common table expressions (CTE), a.k.a. "with" statements
villebro commented on issue #3998: [SQLLab] Allow common table expressions (CTE), a.k.a. "with" statements URL: https://github.com/apache/incubator-superset/issues/3998#issuecomment-377607727 @sangramdhal102 I'm surprised the query runs, as you seem to have mixed ' and ’. Example: ```soapgatewayservice_received >= 'startdate’```. I believe that's what is confusing sqlparse. I'm not an expert in Postgres, but I would perhaps refactor the query to make it slightly easier to read. Something along these lines: ```sql with total_tasks_per_project as ( select extract('hour' from soapgatewayservice_received) as hour, modality, case lower(ingestion_status) when 'completed' then 1 else null end as completed, case lower(ingestion_status) when 'failed' then 1 else null end as failed, case lower(ingestion_status) when 'inprocess' then 1 else null end as inprocess, ingestioncheckpointservice_completed - soapgatewayservice_received as ingestion_time from provenance.provenancedata where soapgatewayservice_received >= 'startdate' and soapgatewayservice_received <= 'enddate' and modality in ('CV','DC','MR','ICAP') union select extract('hour' from restingestiongatewayservice_received) as hour, modality, case lower(ingestion_status) when 'completed' then 1 else null end as completed, case lower(ingestion_status) when 'failed' then 1 else null end as failed, case lower(ingestion_status) when 'inprocess' then 1 else null end as inprocess, ingestioncheckpointservice_completed - restingestiongatewayservice_received as ingestion_time from provenance.provenancedata where restingestiongatewayservice_received >= 'startdate' and restingestiongatewayservice_received <= 'enddate' and modality = 'CT' ) select hour, modality, count(*) as total, count(completed) as completed, count(failed) as failed, count(inprocess) as inprocess, avg(ingestion_time) as avg_ingestion_time from total_tasks_per_project group by hour, modality order by hour ``` This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] villebro commented on issue #3998: [SQLLab] Allow common table expressions (CTE), a.k.a. "with" statements
villebro commented on issue #3998: [SQLLab] Allow common table expressions (CTE), a.k.a. "with" statements URL: https://github.com/apache/incubator-superset/issues/3998#issuecomment-377607727 @sangramdhal102 I'm surprised the query runs, as you seem to have mixed ' and ’. Example: ```soapgatewayservice_received >= 'startdate’```. I believe that's what is confusing sqlparse. I'm not an expert in Postgres, but I would perhaps refactor the query to make it slightly easier to read. Something along these lines: ```sql with total_tasks_per_project as ( select extract('hour' from soapgatewayservice_received) as hour, modality, case lower(ingestion_status) when 'completed' then 1 else null end as completed, case lower(ingestion_status) when 'failed' then 1 else null end as failed, case lower(ingestion_status) when 'inprocess' then 1 else null end as inprocess, ingestioncheckpointservice_completed - soapgatewayservice_received as ingestion_time from provenance.provenancedata where soapgatewayservice_received >= 'startdate' and soapgatewayservice_received <= 'enddate' and modality in ('CV','DC','MR','ICAP') group by modality, hour union select extract('hour' from restingestiongatewayservice_received) as hour, modality, case lower(ingestion_status) when 'completed' then 1 else null end as completed, case lower(ingestion_status) when 'failed' then 1 else null end as failed, case lower(ingestion_status) when 'inprocess' then 1 else null end as inprocess, ingestioncheckpointservice_completed - restingestiongatewayservice_received as ingestion_time from provenance.provenancedata where restingestiongatewayservice_received >= 'startdate' and restingestiongatewayservice_received <= 'enddate' and modality = 'CT' group by modality, hour ) select hour, modality, count(*) as total, count(completed) as completed, count(failed) as failed, count(inprocess) as inprocess, avg(ingestion_time) as avg_ingestion_time from total_tasks_per_project group by hour, modality order by hour ``` This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] alanmcruickshank commented on issue #3972: [feature] Global customise-able d3 locale
alanmcruickshank commented on issue #3972: [feature] Global customise-able d3 locale URL: https://github.com/apache/incubator-superset/issues/3972#issuecomment-377789215 @tristix - no work around as far as I know at the moment. This feature is relatively self contained so could be a good starter-feature if you fancy having a go at it? This is an automated message from the Apache Git Service. To respond to the message, please log on 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
[GitHub] lprashant-94 opened a new pull request #4728: Disabled run query button if sql query editor is empty
lprashant-94 opened a new pull request #4728: Disabled run query button if sql query editor is empty URL: https://github.com/apache/incubator-superset/pull/4728 Fix #2102 Disabled run button if sql query is empty. This is an automated message from the Apache Git Service. To respond to the message, please log on 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