You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/03/10 00:15:34 UTC
[GitHub] [incubator-superset] ktmud opened a new pull request #9269: fix:
upgrade legacy table chart to 0.11.17
ktmud opened a new pull request #9269: fix: upgrade legacy table chart to 0.11.17
URL: https://github.com/apache/incubator-superset/pull/9269
### CATEGORY
Choose one
- [x] Bug Fix
### SUMMARY
https://github.com/apache/incubator-superset/pull/9234 introduced a bug in which the table chart throws errors when rerendering with different query results. It also lacks handling of null values in metrics and percent metrics.
https://github.com/apache-superset/superset-ui-plugins/pull/392 fixed it. This PR bumps plugin dependency to enable this fix.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A
### TEST PLAN
A fresh install of Superset or `npm install` should already fixes the bug because we published a minor new version.
To reproduce the bug, run `npm install @superset-ui/legacy-plugin-chart-table@0.11.16`. Then go to any Table chart, either add or remove a new metric, rerun the query multiple times. Run any query with Table chart multiple times, it should not thrown a JS error like this:
It should not throw this error after the fix.
### ADDITIONAL INFORMATION
None
### REVIEWERS
@kristw @etr2460 @graceguo-supercat
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kristw merged pull request #9269: fix: upgrade
legacy table chart to 0.11.17
Posted by GitBox <gi...@apache.org>.
kristw merged pull request #9269: fix: upgrade legacy table chart to 0.11.17
URL: https://github.com/apache/incubator-superset/pull/9269
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kristw commented on a change in pull request
#9269: fix: upgrade legacy table chart to 0.11.17
Posted by GitBox <gi...@apache.org>.
kristw commented on a change in pull request #9269: fix: upgrade legacy table chart to 0.11.17
URL: https://github.com/apache/incubator-superset/pull/9269#discussion_r390037764
##########
File path: superset-frontend/package-lock.json
##########
@@ -7689,28 +7670,28 @@
"dependencies": {
"abbrev": {
"version": "1.1.1",
- "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz",
+ "resolved": false,
Review comment:
Let me try
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kristw commented on a change in pull request
#9269: fix: upgrade legacy table chart to 0.11.17
Posted by GitBox <gi...@apache.org>.
kristw commented on a change in pull request #9269: fix: upgrade legacy table chart to 0.11.17
URL: https://github.com/apache/incubator-superset/pull/9269#discussion_r390043054
##########
File path: superset-frontend/package-lock.json
##########
@@ -7689,28 +7670,28 @@
"dependencies": {
"abbrev": {
"version": "1.1.1",
- "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz",
+ "resolved": false,
Review comment:
yeah. did `npm install` also see `false`
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud commented on a change in pull request
#9269: fix: upgrade legacy table chart to 0.11.17
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #9269: fix: upgrade legacy table chart to 0.11.17
URL: https://github.com/apache/incubator-superset/pull/9269#discussion_r390030435
##########
File path: superset-frontend/package-lock.json
##########
@@ -7689,28 +7670,28 @@
"dependencies": {
"abbrev": {
"version": "1.1.1",
- "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz",
+ "resolved": false,
Review comment:
I don't understand why a clean install `npm ci` would update `resolved` to `false`. A quick google suggests `npm >= 5.0.0` will [simply ignore this field](https://stackoverflow.com/questions/53814967/what-is-the-point-of-having-resolved-url-in-package-lock-json). Did someone updated `pcakge-lock.json` with a lower version of `npm`?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kristw commented on a change in pull request
#9269: fix: upgrade legacy table chart to 0.11.17
Posted by GitBox <gi...@apache.org>.
kristw commented on a change in pull request #9269: fix: upgrade legacy table chart to 0.11.17
URL: https://github.com/apache/incubator-superset/pull/9269#discussion_r390033857
##########
File path: superset-frontend/package-lock.json
##########
@@ -7689,28 +7670,28 @@
"dependencies": {
"abbrev": {
"version": "1.1.1",
- "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz",
+ "resolved": false,
Review comment:
so weird. I use `npm@6`
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io commented on issue #9269: fix:
upgrade legacy table chart to 0.11.17
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #9269: fix: upgrade legacy table chart to 0.11.17
URL: https://github.com/apache/incubator-superset/pull/9269#issuecomment-596883979
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9269?src=pr&el=h1) Report
> Merging [#9269](https://codecov.io/gh/apache/incubator-superset/pull/9269?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/d8fea640f89160f8aeccfcc3702f1fc39ae08fd8?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9269?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9269 +/- ##
======================================
Coverage 58.9% 58.9%
======================================
Files 373 373
Lines 12026 12026
Branches 2953 2953
======================================
Hits 7084 7084
Misses 4763 4763
Partials 179 179
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9269?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9269?src=pr&el=footer). Last update [d8fea64...2b497c6](https://codecov.io/gh/apache/incubator-superset/pull/9269?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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud commented on a change in pull request
#9269: fix: upgrade legacy table chart to 0.11.17
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #9269: fix: upgrade legacy table chart to 0.11.17
URL: https://github.com/apache/incubator-superset/pull/9269#discussion_r390035863
##########
File path: superset-frontend/package-lock.json
##########
@@ -7689,28 +7670,28 @@
"dependencies": {
"abbrev": {
"version": "1.1.1",
- "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz",
+ "resolved": false,
Review comment:
@kristw Do you get the same package-lock as me when you install?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] etr2460 commented on a change in pull request
#9269: fix: upgrade legacy table chart to 0.11.17
Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #9269: fix: upgrade legacy table chart to 0.11.17
URL: https://github.com/apache/incubator-superset/pull/9269#discussion_r390044747
##########
File path: superset-frontend/package-lock.json
##########
@@ -7689,28 +7670,28 @@
"dependencies": {
"abbrev": {
"version": "1.1.1",
- "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz",
+ "resolved": false,
Review comment:
i'm on 6.14.2, so not sure 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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org