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/07/09 05:33:42 UTC

[GitHub] [incubator-superset] mistercrunch opened a new pull request #10267: fix: broken glyphicons used in react-json-schema

mistercrunch opened a new pull request #10267:
URL: https://github.com/apache/incubator-superset/pull/10267


   I'm working on a talk for the Airflow Summit
   "Advanced Apache Superset for Data Engineers" and showing the "Schedule
   Query" feature that Beto contributed a while back (behind a feature flag).
   
   I found that the glyphicons used in `react-json-schema` are broken and
   came up with an easy fix.
   
   Also other minor tweaks on the feature.
   


----------------------------------------------------------------
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



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10267: fix: broken glyphicons used in react-json-schema

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10267:
URL: https://github.com/apache/incubator-superset/pull/10267#issuecomment-655918789


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10267?src=pr&el=h1) Report
   > Merging [#10267](https://codecov.io/gh/apache/incubator-superset/pull/10267?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/3dfa4aa15d720bbcbb4fc7dc6c296c32fe119bbd&el=desc) will **decrease** coverage by `4.89%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/10267/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/10267?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #10267      +/-   ##
   ==========================================
   - Coverage   70.18%   65.28%   -4.90%     
   ==========================================
     Files         598      598              
     Lines       32004    31994      -10     
     Branches     3236     3236              
   ==========================================
   - Hits        22462    20888    -1574     
   - Misses       9437    10925    +1488     
   - Partials      105      181      +76     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | #cypress | `?` | |
   | #javascript | `59.28% <ø> (ø)` | |
   | #python | `69.46% <ø> (-0.22%)` | :arrow_down: |
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10267?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...tend/src/SqlLab/components/ScheduleQueryButton.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10267/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NjaGVkdWxlUXVlcnlCdXR0b24uanN4) | `8.62% <ø> (ø)` | |
   | [superset/views/sql\_lab.py](https://codecov.io/gh/apache/incubator-superset/pull/10267/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3Mvc3FsX2xhYi5weQ==) | `62.57% <ø> (ø)` | |
   | [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10267/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10267/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [superset-frontend/src/SqlLab/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10267/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10267/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10267/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10267/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/10267/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [superset-frontend/src/chart/ChartContainer.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10267/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NoYXJ0L0NoYXJ0Q29udGFpbmVyLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | ... and [154 more](https://codecov.io/gh/apache/incubator-superset/pull/10267/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10267?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/10267?src=pr&el=footer). Last update [3dfa4aa...42bb6b7](https://codecov.io/gh/apache/incubator-superset/pull/10267?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



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [incubator-superset] mistercrunch commented on a change in pull request #10267: fix: broken glyphicons used in react-json-schema

Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #10267:
URL: https://github.com/apache/incubator-superset/pull/10267#discussion_r454623379



##########
File path: superset/views/sql_lab.py
##########
@@ -162,6 +162,7 @@ class SavedQueryViewApi(SavedQueryView):  # pylint: disable=too-many-ancestors
         "description",
         "sql",
         "extra_json",
+        "extra",

Review comment:
       `extra_json` is a string containing the serialized JSON, `extra` is the actual nested data structure. While I know the private API does not use `extra_json`, I assume that people in the wild (notably at Lyft where @betodealmeida shipped the feature) are very likely to use it. 
   
   Clearly it's not great to have JSON inception (serialized JSON inside of JSON)
   
   I could have deprecated `extra_json` in favor of `extra`, but would need to do change management around it - likely just adding a note in `UPDATING.md`. Eventually we should move this under `/api/v1` but, maybe a good time to do 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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [incubator-superset] codecov-commenter commented on pull request #10267: fix: broken glyphicons used in react-json-schema

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #10267:
URL: https://github.com/apache/incubator-superset/pull/10267#issuecomment-655918789


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10267?src=pr&el=h1) Report
   > Merging [#10267](https://codecov.io/gh/apache/incubator-superset/pull/10267?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/3dfa4aa15d720bbcbb4fc7dc6c296c32fe119bbd&el=desc) will **decrease** coverage by `0.71%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/10267/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/10267?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #10267      +/-   ##
   ==========================================
   - Coverage   70.18%   69.46%   -0.72%     
   ==========================================
     Files         598      194     -404     
     Lines       32004    18853   -13151     
     Branches     3236        0    -3236     
   ==========================================
   - Hits        22462    13097    -9365     
   + Misses       9437     5756    -3681     
   + Partials      105        0     -105     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | #cypress | `?` | |
   | #javascript | `?` | |
   | #python | `69.46% <ø> (-0.22%)` | :arrow_down: |
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10267?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset/views/sql\_lab.py](https://codecov.io/gh/apache/incubator-superset/pull/10267/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3Mvc3FsX2xhYi5weQ==) | `62.57% <ø> (ø)` | |
   | [superset/views/database/mixins.py](https://codecov.io/gh/apache/incubator-superset/pull/10267/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `59.64% <0.00%> (-21.06%)` | :arrow_down: |
   | [superset/utils/cache.py](https://codecov.io/gh/apache/incubator-superset/pull/10267/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2FjaGUucHk=) | `48.00% <0.00%> (-20.00%)` | :arrow_down: |
   | [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/incubator-superset/pull/10267/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `78.72% <0.00%> (-12.77%)` | :arrow_down: |
   | [superset/views/database/validators.py](https://codecov.io/gh/apache/incubator-superset/pull/10267/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvdmFsaWRhdG9ycy5weQ==) | `78.94% <0.00%> (-5.27%)` | :arrow_down: |
   | [superset/views/database/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10267/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvYXBpLnB5) | `84.09% <0.00%> (-3.41%)` | :arrow_down: |
   | [superset/db\_engine\_specs/postgres.py](https://codecov.io/gh/apache/incubator-superset/pull/10267/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3Bvc3RncmVzLnB5) | `97.36% <0.00%> (-2.64%)` | :arrow_down: |
   | [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10267/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `84.44% <0.00%> (-2.23%)` | :arrow_down: |
   | [superset/views/database/views.py](https://codecov.io/gh/apache/incubator-superset/pull/10267/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2Uvdmlld3MucHk=) | `61.32% <0.00%> (-1.66%)` | :arrow_down: |
   | [superset/jinja\_context.py](https://codecov.io/gh/apache/incubator-superset/pull/10267/diff?src=pr&el=tree#diff-c3VwZXJzZXQvamluamFfY29udGV4dC5weQ==) | `80.00% <0.00%> (-0.96%)` | :arrow_down: |
   | ... and [409 more](https://codecov.io/gh/apache/incubator-superset/pull/10267/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10267?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/10267?src=pr&el=footer). Last update [3dfa4aa...42bb6b7](https://codecov.io/gh/apache/incubator-superset/pull/10267?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



---------------------------------------------------------------------
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 pull request #10267: fix: broken glyphicons used in react-json-schema

Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #10267:
URL: https://github.com/apache/incubator-superset/pull/10267#issuecomment-655964573


   On a separate note, we can probably start thinking about replacing all code editor instances with [monaco-editor](https://github.com/microsoft/monaco-editor), the editor behind VS Code, which also supports json schemas. It's much more actively maintained than Ace/Brace and has many more advanced features.


----------------------------------------------------------------
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



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [incubator-superset] willbarrett commented on a change in pull request #10267: fix: broken glyphicons used in react-json-schema

Posted by GitBox <gi...@apache.org>.
willbarrett commented on a change in pull request #10267:
URL: https://github.com/apache/incubator-superset/pull/10267#discussion_r454601762



##########
File path: superset/views/sql_lab.py
##########
@@ -162,6 +162,7 @@ class SavedQueryViewApi(SavedQueryView):  # pylint: disable=too-many-ancestors
         "description",
         "sql",
         "extra_json",
+        "extra",

Review comment:
       I'm surprised to see both `extra` and `extra_json` - what's the difference/motivation?




----------------------------------------------------------------
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



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [incubator-superset] mistercrunch merged pull request #10267: fix: broken glyphicons used in react-json-schema

Posted by GitBox <gi...@apache.org>.
mistercrunch merged pull request #10267:
URL: https://github.com/apache/incubator-superset/pull/10267


   


----------------------------------------------------------------
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



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org