[GitHub] mistercrunch opened a new pull request #4696: [sql lab] preserve schema on visualize flow

2018-03-26 Thread GitBox
mistercrunch opened a new pull request #4696: [sql lab] preserve schema on 
visualize flow
URL: https://github.com/apache/incubator-superset/pull/4696
 
 
   When following the visualize flow, if SQL Lab is pointing to the non-default 
schema, and the user doesn't specify the schema in their query as in `FROM 
some_schema.some_table`, the flow just breaks with a message like `The table 
does not exist` or something of that nature depending on the engine.
   
   This PR fixes this + related refactors:
   * moving most SQL Lab views into views/sqllab.py
   * corresponding redirect for backward compatibility
   * new unit test for the backend part of the visualize flow
   * fixing objects that were double-serialized to json for no good reason


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 #4693: Use 3 letters month prefix in default date format

2018-03-26 Thread GitBox
codecov-io commented on issue #4693: Use 3 letters month prefix in default date 
format
URL: 
https://github.com/apache/incubator-superset/pull/4693#issuecomment-376398937
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4693?src=pr=h1)
 Report
   > Merging 
[#4693](https://codecov.io/gh/apache/incubator-superset/pull/4693?src=pr=desc)
 into 
[master](https://codecov.io/gh/apache/incubator-superset/commit/f9d85bd2e1fd9bc233d19c76bed09467522b968a?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/4693/graphs/tree.svg?width=650=KsB0fHcx6l=150=pr)](https://codecov.io/gh/apache/incubator-superset/pull/4693?src=pr=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master#4693  +/-   ##
   ==
   + Coverage   71.52%   71.53%   +<.01% 
   ==
 Files 190  190  
 Lines   1492714929   +2 
 Branches 1102 1102  
   ==
   + Hits1067710679   +2 
 Misses   4247 4247  
 Partials33
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-superset/pull/4693?src=pr=tree)
 | Coverage Δ | |
   |---|---|---|
   | 
[superset/assets/javascripts/modules/dates.js](https://codecov.io/gh/apache/incubator-superset/pull/4693/diff?src=pr=tree#diff-c3VwZXJzZXQvYXNzZXRzL2phdmFzY3JpcHRzL21vZHVsZXMvZGF0ZXMuanM=)
 | `86.04% <100%> (+0.68%)` | :arrow_up: |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4693?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/4693?src=pr=footer).
 Last update 
[f9d85bd...9bb00b2](https://codecov.io/gh/apache/incubator-superset/pull/4693?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] yamyamyuo commented on issue #1935: UnicodeEncodeError when show slice record

2018-03-26 Thread GitBox
yamyamyuo commented on issue #1935: UnicodeEncodeError when show slice record
URL: 
https://github.com/apache/incubator-superset/issues/1935#issuecomment-376384200
 
 
   I have solved this problem. I am using py2. 
   1. you should fix mysql url by 
`mysql://user:password@1.1.1.1/db?charset=utf8`
   2. if you are using py2, you should specify 
   ```
   if sys.version_info.major < 3:
   reload(sys)
   sys.setdefaultencoding('utf8')
   ```
   and then things works ☄️


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 #4565: [security] Refactor security code into SupersetSecurityManager

2018-03-26 Thread GitBox
codecov-io commented on issue #4565: [security] Refactor security code into 
SupersetSecurityManager
URL: 
https://github.com/apache/incubator-superset/pull/4565#issuecomment-373476840
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4565?src=pr=h1)
 Report
   > Merging 
[#4565](https://codecov.io/gh/apache/incubator-superset/pull/4565?src=pr=desc)
 into 
[master](https://codecov.io/gh/apache/incubator-superset/commit/f9d85bd2e1fd9bc233d19c76bed09467522b968a?src=pr=desc)
 will **decrease** coverage by `0.01%`.
   > The diff coverage is `71.96%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-superset/pull/4565/graphs/tree.svg?token=KsB0fHcx6l=pr=150=650)](https://codecov.io/gh/apache/incubator-superset/pull/4565?src=pr=tree)
   
   ```diff
   @@Coverage Diff@@
   ##   master   #4565  +/-   ##
   =
   - Coverage   71.52%   71.5%   -0.02% 
   =
 Files 190 190  
 Lines   14927   14935   +8 
 Branches 11021102  
   =
   + Hits10677   10680   +3 
   - Misses   42474252   +5 
 Partials3   3
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-superset/pull/4565?src=pr=tree)
 | Coverage Δ | |
   |---|---|---|
   | 
[superset/\_\_init\_\_.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvX19pbml0X18ucHk=)
 | `72.97% <100%> (+1.01%)` | :arrow_up: |
   | 
[superset/views/base.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvdmlld3MvYmFzZS5weQ==)
 | `67.12% <100%> (+8.86%)` | :arrow_up: |
   | 
[superset/data/\_\_init\_\_.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvZGF0YS9fX2luaXRfXy5weQ==)
 | `100% <100%> (ø)` | :arrow_up: |
   | 
[superset/connectors/sqla/models.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==)
 | `77.91% <100%> (ø)` | :arrow_up: |
   | 
[superset/utils.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvdXRpbHMucHk=)
 | `88.12% <100%> (+0.15%)` | :arrow_up: |
   | 
[superset/connectors/druid/models.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9kcnVpZC9tb2RlbHMucHk=)
 | `78.85% <100%> (ø)` | :arrow_up: |
   | 
[superset/connectors/druid/views.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9kcnVpZC92aWV3cy5weQ==)
 | `68.02% <16.66%> (ø)` | :arrow_up: |
   | 
[superset/connectors/sqla/views.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL3ZpZXdzLnB5)
 | `71.56% <20%> (ø)` | :arrow_up: |
   | 
[superset/cli.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvY2xpLnB5)
 | `47.74% <33.33%> (-0.65%)` | :arrow_down: |
   | 
[superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==)
 | `71.17% <61.11%> (-0.03%)` | :arrow_down: |
   | ... and [1 
more](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree-more)
 | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4565?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/4565?src=pr=footer).
 Last update 
[f9d85bd...a385444](https://codecov.io/gh/apache/incubator-superset/pull/4565?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] timifasubaa commented on a change in pull request #4565: [security] Refactor security code into SupersetSecurityManager

2018-03-26 Thread GitBox
timifasubaa commented on a change in pull request #4565: [security] Refactor 
security code into SupersetSecurityManager
URL: 
https://github.com/apache/incubator-superset/pull/4565#discussion_r177290855
 
 

 ##
 File path: superset/data/__init__.py
 ##
 @@ -71,7 +70,7 @@ def load_energy():
 if not tbl:
 tbl = TBL(table_name=tbl_name)
 tbl.description = "Energy consumption"
-tbl.database = get_or_create_main_db()
+tbl.database = sm.get_or_create_main_db()
 
 Review comment:
   My main concern is that moving this method will also make me move other 
things and make this already big PR even bigger. Moreover, since the method was 
already in security.py, it is not too far fetched to leave it there for now. 


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 a change in pull request #4565: [security] Refactor security code into SupersetSecurityManager

2018-03-26 Thread GitBox
timifasubaa commented on a change in pull request #4565: [security] Refactor 
security code into SupersetSecurityManager
URL: 
https://github.com/apache/incubator-superset/pull/4565#discussion_r177290855
 
 

 ##
 File path: superset/data/__init__.py
 ##
 @@ -71,7 +70,7 @@ def load_energy():
 if not tbl:
 tbl = TBL(table_name=tbl_name)
 tbl.description = "Energy consumption"
-tbl.database = get_or_create_main_db()
+tbl.database = sm.get_or_create_main_db()
 
 Review comment:
   My main concern is that moving this method will also make me move other 
things and make this PR even bigger and since it was already in security.py, it 
is not too far fetched to leave it there for now. 


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] john-bodley commented on a change in pull request #4565: [security] Refactor security code into SupersetSecurityManager

2018-03-26 Thread GitBox
john-bodley commented on a change in pull request #4565: [security] Refactor 
security code into SupersetSecurityManager
URL: 
https://github.com/apache/incubator-superset/pull/4565#discussion_r177285484
 
 

 ##
 File path: superset/data/__init__.py
 ##
 @@ -71,7 +70,7 @@ def load_energy():
 if not tbl:
 tbl = TBL(table_name=tbl_name)
 tbl.description = "Energy consumption"
-tbl.database = get_or_create_main_db()
+tbl.database = sm.get_or_create_main_db()
 
 Review comment:
   Shouldn't this PR address this specific issue, especially as from this PR it 
would now indicate that the `get_or_create_main_db()` is tightly coupled with 
the security manager.  


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 #4565: [security] Refactor security code into SupersetSecurityManager

2018-03-26 Thread GitBox
codecov-io commented on issue #4565: [security] Refactor security code into 
SupersetSecurityManager
URL: 
https://github.com/apache/incubator-superset/pull/4565#issuecomment-373476840
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4565?src=pr=h1)
 Report
   > Merging 
[#4565](https://codecov.io/gh/apache/incubator-superset/pull/4565?src=pr=desc)
 into 
[master](https://codecov.io/gh/apache/incubator-superset/commit/52b925fee80c0d46aeb444326ac499296d682396?src=pr=desc)
 will **decrease** coverage by `<.01%`.
   > The diff coverage is `72.8%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-superset/pull/4565/graphs/tree.svg?width=650=150=pr=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/4565?src=pr=tree)
   
   ```diff
   @@Coverage Diff@@
   ##   master   #4565  +/-   ##
   =
   - Coverage71.4%   71.4%   -0.01% 
   =
 Files 190 190  
 Lines   14934   14943   +9 
 Branches 11021102  
   =
   + Hits10664   10670   +6 
   - Misses   42674270   +3 
 Partials3   3
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-superset/pull/4565?src=pr=tree)
 | Coverage Δ | |
   |---|---|---|
   | 
[superset/utils.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvdXRpbHMucHk=)
 | `87.77% <ø> (-0.19%)` | :arrow_down: |
   | 
[superset/data/\_\_init\_\_.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvZGF0YS9fX2luaXRfXy5weQ==)
 | `100% <100%> (ø)` | :arrow_up: |
   | 
[superset/views/base.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvdmlld3MvYmFzZS5weQ==)
 | `67.12% <100%> (+8.86%)` | :arrow_up: |
   | 
[superset/\_\_init\_\_.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvX19pbml0X18ucHk=)
 | `72.97% <100%> (+1.01%)` | :arrow_up: |
   | 
[superset/connectors/druid/models.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9kcnVpZC9tb2RlbHMucHk=)
 | `76.35% <100%> (ø)` | :arrow_up: |
   | 
[superset/connectors/sqla/models.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==)
 | `77.91% <100%> (ø)` | :arrow_up: |
   | 
[superset/connectors/druid/views.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9kcnVpZC92aWV3cy5weQ==)
 | `68.02% <16.66%> (ø)` | :arrow_up: |
   | 
[superset/connectors/sqla/views.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL3ZpZXdzLnB5)
 | `71.56% <20%> (ø)` | :arrow_up: |
   | 
[superset/cli.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvY2xpLnB5)
 | `48.38% <50%> (ø)` | :arrow_up: |
   | 
[superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==)
 | `71.17% <61.11%> (-0.03%)` | :arrow_down: |
   | ... and [1 
more](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree-more)
 | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4565?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/4565?src=pr=footer).
 Last update 
[52b925f...7c74dec](https://codecov.io/gh/apache/incubator-superset/pull/4565?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 #4565: [security] Refactor security code into SupersetSecurityManager

2018-03-26 Thread GitBox
codecov-io commented on issue #4565: [security] Refactor security code into 
SupersetSecurityManager
URL: 
https://github.com/apache/incubator-superset/pull/4565#issuecomment-373476840
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4565?src=pr=h1)
 Report
   > Merging 
[#4565](https://codecov.io/gh/apache/incubator-superset/pull/4565?src=pr=desc)
 into 
[master](https://codecov.io/gh/apache/incubator-superset/commit/52b925fee80c0d46aeb444326ac499296d682396?src=pr=desc)
 will **decrease** coverage by `<.01%`.
   > The diff coverage is `72.8%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-superset/pull/4565/graphs/tree.svg?width=650=pr=KsB0fHcx6l=150)](https://codecov.io/gh/apache/incubator-superset/pull/4565?src=pr=tree)
   
   ```diff
   @@Coverage Diff@@
   ##   master   #4565  +/-   ##
   =
   - Coverage71.4%   71.4%   -0.01% 
   =
 Files 190 190  
 Lines   14934   14943   +9 
 Branches 11021102  
   =
   + Hits10664   10670   +6 
   - Misses   42674270   +3 
 Partials3   3
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-superset/pull/4565?src=pr=tree)
 | Coverage Δ | |
   |---|---|---|
   | 
[superset/utils.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvdXRpbHMucHk=)
 | `87.77% <ø> (-0.19%)` | :arrow_down: |
   | 
[superset/connectors/sqla/models.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==)
 | `77.91% <100%> (ø)` | :arrow_up: |
   | 
[superset/connectors/druid/models.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9kcnVpZC9tb2RlbHMucHk=)
 | `76.35% <100%> (ø)` | :arrow_up: |
   | 
[superset/data/\_\_init\_\_.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvZGF0YS9fX2luaXRfXy5weQ==)
 | `100% <100%> (ø)` | :arrow_up: |
   | 
[superset/\_\_init\_\_.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvX19pbml0X18ucHk=)
 | `72.97% <100%> (+1.01%)` | :arrow_up: |
   | 
[superset/views/base.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvdmlld3MvYmFzZS5weQ==)
 | `67.12% <100%> (+8.86%)` | :arrow_up: |
   | 
[superset/connectors/druid/views.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9kcnVpZC92aWV3cy5weQ==)
 | `68.02% <16.66%> (ø)` | :arrow_up: |
   | 
[superset/connectors/sqla/views.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL3ZpZXdzLnB5)
 | `71.56% <20%> (ø)` | :arrow_up: |
   | 
[superset/cli.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvY2xpLnB5)
 | `48.38% <50%> (ø)` | :arrow_up: |
   | 
[superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==)
 | `71.17% <61.11%> (-0.03%)` | :arrow_down: |
   | ... and [1 
more](https://codecov.io/gh/apache/incubator-superset/pull/4565/diff?src=pr=tree-more)
 | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4565?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/4565?src=pr=footer).
 Last update 
[52b925f...7c74dec](https://codecov.io/gh/apache/incubator-superset/pull/4565?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] darylerwin opened a new issue #4695: When does 0.24.0 appear on Pypi?

2018-03-26 Thread GitBox
darylerwin opened a new issue #4695: When does 0.24.0 appear on Pypi?
URL: https://github.com/apache/incubator-superset/issues/4695
 
 
   Make sure these boxes are checked before submitting your issue - thank you!
   
   - [ ] I have checked the superset logs for python stacktraces and included 
it here as text if any
   - [ ] I have reproduced the issue with at least the latest released version 
of superset
   - [ ] I have checked the issue tracker for the same issue and I haven't 
found one similar
   
   
   ### Superset version
   0.23.3
   
   
   Appears the tarball is there .. so is there a step/reason it is not 
available 
   (sorry for my ignorance on how this works)
   https://github.com/airbnb/superset/tarball/0.24.0
   
   https://pypi.python.org/pypi/superset/0.23.3
   
   I assume this is needed for
   pip install --upgrade superset to 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] john-bodley closed pull request #4655: [druid] Updating refresh logic

2018-03-26 Thread GitBox
john-bodley closed pull request #4655: [druid] Updating refresh logic
URL: https://github.com/apache/incubator-superset/pull/4655
 
 
   

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/cli.py b/superset/cli.py
index 48db7394b9..95f6df7b95 100755
--- a/superset/cli.py
+++ b/superset/cli.py
@@ -166,10 +166,9 @@ def load_examples(load_test_data):
 )
 @manager.option(
 '-m', '--merge',
-help=(
-"Specify using 'merge' property during operation. "
-'Default value is False '
-),
+action='store_true',
+help="Specify using 'merge' property during operation.",
+default=False,
 )
 def refresh_druid(datasource, merge):
 """Refresh druid datasources"""
diff --git a/superset/connectors/druid/models.py 
b/superset/connectors/druid/models.py
index 74eb575d04..f4f137ef2f 100644
--- a/superset/connectors/druid/models.py
+++ b/superset/connectors/druid/models.py
@@ -29,7 +29,7 @@
 from six import string_types
 import sqlalchemy as sa
 from sqlalchemy import (
-Boolean, Column, DateTime, ForeignKey, Integer, or_, String, Text, 
UniqueConstraint,
+Boolean, Column, DateTime, ForeignKey, Integer, String, Text, 
UniqueConstraint,
 )
 from sqlalchemy.orm import backref, relationship
 
@@ -200,33 +200,31 @@ def refresh(self, datasource_names, merge_flag, 
refreshAll):
 col_objs_list = (
 session.query(DruidColumn)
 .filter(DruidColumn.datasource_id == datasource.id)
-.filter(or_(DruidColumn.column_name == col for col in 
cols))
+.filter(DruidColumn.column_name.in_(cols.keys()))
 )
 col_objs = {col.column_name: col for col in col_objs_list}
 for col in cols:
 if col == '__time':  # skip the time column
 continue
-col_obj = col_objs.get(col, None)
+col_obj = col_objs.get(col)
 if not col_obj:
 col_obj = DruidColumn(
 datasource_id=datasource.id,
 column_name=col)
 with session.no_autoflush:
 session.add(col_obj)
-datatype = cols[col]['type']
-if datatype == 'STRING':
+col_obj.type = cols[col]['type']
+col_obj.datasource = datasource
+if col_obj.type == 'STRING':
 col_obj.groupby = True
 col_obj.filterable = True
-if datatype == 'hyperUnique' or datatype == 'thetaSketch':
+if col_obj.type == 'hyperUnique' or col_obj.type == 
'thetaSketch':
 col_obj.count_distinct = True
-# Allow sum/min/max for long or double
-if datatype == 'LONG' or datatype == 'DOUBLE':
+if col_obj.is_num:
 col_obj.sum = True
 col_obj.min = True
 col_obj.max = True
-col_obj.type = datatype
-col_obj.datasource = datasource
-datasource.generate_metrics_for(col_objs_list)
+datasource.refresh_metrics()
 session.commit()
 
 @property
@@ -361,21 +359,24 @@ def get_metrics(self):
 )
 return metrics
 
-def generate_metrics(self):
-"""Generate metrics based on the column metadata"""
+def refresh_metrics(self):
+"""Refresh metrics based on the column metadata"""
 metrics = self.get_metrics()
 dbmetrics = (
 db.session.query(DruidMetric)
 .filter(DruidMetric.datasource_id == self.datasource_id)
-.filter(or_(
-DruidMetric.metric_name == m for m in metrics
-))
+.filter(DruidMetric.metric_name.in_(metrics.keys()))
 )
 dbmetrics = {metric.metric_name: metric for metric in dbmetrics}
 for metric in metrics.values():
-metric.datasource_id = self.datasource_id
-if not dbmetrics.get(metric.metric_name, None):
-db.session.add(metric)
+dbmetric = dbmetrics.get(metric.metric_name)
+if dbmetric:
+for attr in ['json', 'metric_type', 'verbose_name']:
+setattr(dbmetric, attr, getattr(metric, attr))
+else:
+with db.session.no_autoflush:
+metric.datasource_id = self.datasource_id
+db.session.add(metric)
 
 @classmethod
 def import_obj(cls, i_column):
@@ -653,24 +654,9 @@ def 

[GitHub] timifasubaa commented on a change in pull request #4565: [security] Refactor security code into SupersetSecurityManager

2018-03-26 Thread GitBox
timifasubaa commented on a change in pull request #4565: [security] Refactor 
security code into SupersetSecurityManager
URL: 
https://github.com/apache/incubator-superset/pull/4565#discussion_r177267268
 
 

 ##
 File path: superset/__init__.py
 ##
 @@ -149,12 +150,16 @@ def index(self):
 return redirect('/superset/welcome')
 
 
+custom_sm = app.config.get('CUSTOM_SECURITY_MANAGER')
 
 Review comment:
   I found out that this only works when the variable doesn't exist. In this 
case, it is set to be None and so it returns None. 


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 a change in pull request #4565: [security] Refactor security code into SupersetSecurityManager

2018-03-26 Thread GitBox
timifasubaa commented on a change in pull request #4565: [security] Refactor 
security code into SupersetSecurityManager
URL: 
https://github.com/apache/incubator-superset/pull/4565#discussion_r177267268
 
 

 ##
 File path: superset/__init__.py
 ##
 @@ -149,12 +150,16 @@ def index(self):
 return redirect('/superset/welcome')
 
 
+custom_sm = app.config.get('CUSTOM_SECURITY_MANAGER')
 
 Review comment:
   I found out that this only works when the variable doesn't exist. In this 
case, it is set to be null and so it returns null. 


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 a change in pull request #4565: [security] Refactor security code into SupersetSecurityManager

2018-03-26 Thread GitBox
timifasubaa commented on a change in pull request #4565: [security] Refactor 
security code into SupersetSecurityManager
URL: 
https://github.com/apache/incubator-superset/pull/4565#discussion_r177277681
 
 

 ##
 File path: superset/data/__init__.py
 ##
 @@ -71,7 +70,7 @@ def load_energy():
 if not tbl:
 tbl = TBL(table_name=tbl_name)
 tbl.description = "Energy consumption"
-tbl.database = get_or_create_main_db()
+tbl.database = sm.get_or_create_main_db()
 
 Review comment:
   Done


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 #4694: [sql lab] schema doesn't carry through in the visualize flow

2018-03-26 Thread GitBox
mistercrunch opened a new issue #4694: [sql lab] schema doesn't carry through 
in the visualize flow
URL: https://github.com/apache/incubator-superset/issues/4694
 
 
   


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 a change in pull request #4565: [security] Refactor security code into SupersetSecurityManager

2018-03-26 Thread GitBox
timifasubaa commented on a change in pull request #4565: [security] Refactor 
security code into SupersetSecurityManager
URL: 
https://github.com/apache/incubator-superset/pull/4565#discussion_r177277941
 
 

 ##
 File path: superset/security.py
 ##
 @@ -77,177 +78,303 @@
 ])
 
 
-def merge_perm(sm, permission_name, view_menu_name):
-# Implementation copied from sm.find_permission_view_menu.
-# TODO: use sm.find_permission_view_menu once issue
-#   https://github.com/airbnb/superset/issues/1944 is resolved.
-permission = sm.find_permission(permission_name)
-view_menu = sm.find_view_menu(view_menu_name)
-pv = None
-if permission and view_menu:
-pv = sm.get_session.query(sm.permissionview_model).filter_by(
-permission=permission, view_menu=view_menu).first()
-if not pv and permission_name and view_menu_name:
-sm.add_permission_view_menu(permission_name, view_menu_name)
-
-
-def is_user_defined_permission(perm):
-return perm.permission.name in OBJECT_SPEC_PERMISSIONS
-
-
-def get_or_create_main_db():
-logging.info('Creating database reference')
-dbobj = (
-db.session.query(models.Database)
-.filter_by(database_name='main')
-.first()
-)
-if not dbobj:
-dbobj = models.Database(database_name='main')
-dbobj.set_sqlalchemy_uri(conf.get('SQLALCHEMY_DATABASE_URI'))
-dbobj.expose_in_sqllab = True
-dbobj.allow_run_sync = True
-db.session.add(dbobj)
-db.session.commit()
-return dbobj
-
-
-def is_admin_only(pvm):
-# not readonly operations on read only model views allowed only for admins
-if (pvm.view_menu.name in READ_ONLY_MODEL_VIEWS and
-pvm.permission.name not in READ_ONLY_PERMISSION):
-return True
-return (
-pvm.view_menu.name in ADMIN_ONLY_VIEW_MENUS or
-pvm.permission.name in ADMIN_ONLY_PERMISSIONS
-)
-
-
-def is_alpha_only(pvm):
-if (pvm.view_menu.name in GAMMA_READ_ONLY_MODEL_VIEWS and
-pvm.permission.name not in READ_ONLY_PERMISSION):
-return True
-return (
-pvm.view_menu.name in ALPHA_ONLY_VIEW_MENUS or
-pvm.permission.name in ALPHA_ONLY_PERMISSIONS
-)
-
-
-def is_admin_pvm(pvm):
-return not is_user_defined_permission(pvm)
-
-
-def is_alpha_pvm(pvm):
-return not (is_user_defined_permission(pvm) or is_admin_only(pvm))
-
-
-def is_gamma_pvm(pvm):
-return not (is_user_defined_permission(pvm) or is_admin_only(pvm) or
-is_alpha_only(pvm))
-
-
-def is_sql_lab_pvm(pvm):
-return pvm.view_menu.name in {'SQL Lab'} or pvm.permission.name in {
-'can_sql_json', 'can_csv', 'can_search_queries',
-}
-
-
-def is_granter_pvm(pvm):
-return pvm.permission.name in {
-'can_override_role_permissions', 'can_approve',
-}
-
-
-def set_role(role_name, pvm_check):
-logging.info('Syncing {} perms'.format(role_name))
-sesh = sm.get_session()
-pvms = sesh.query(ab_models.PermissionView).all()
-pvms = [p for p in pvms if p.permission and p.view_menu]
-role = sm.add_role(role_name)
-role_pvms = [p for p in pvms if pvm_check(p)]
-role.permissions = role_pvms
-sesh.merge(role)
-sesh.commit()
-
-
-def create_custom_permissions():
-# Global perms
-merge_perm(sm, 'all_datasource_access', 'all_datasource_access')
-merge_perm(sm, 'all_database_access', 'all_database_access')
-
-
-def create_missing_perms():
-"""Creates missing perms for datasources, schemas and metrics"""
-
-logging.info(
-'Fetching a set of all perms to lookup which ones are missing')
-all_pvs = set()
-for pv in sm.get_session.query(sm.permissionview_model).all():
-if pv.permission and pv.view_menu:
-all_pvs.add((pv.permission.name, pv.view_menu.name))
-
-def merge_pv(view_menu, perm):
-"""Create permission view menu only if it doesn't exist"""
-if view_menu and perm and (view_menu, perm) not in all_pvs:
-merge_perm(sm, view_menu, perm)
-
-logging.info('Creating missing datasource permissions.')
-datasources = ConnectorRegistry.get_all_datasources(db.session)
-for datasource in datasources:
-merge_pv('datasource_access', datasource.get_perm())
-merge_pv('schema_access', datasource.schema_perm)
-
-logging.info('Creating missing database permissions.')
-databases = db.session.query(models.Database).all()
-for database in databases:
-merge_pv('database_access', database.perm)
-
-logging.info('Creating missing metrics permissions')
-metrics = []
-for datasource_class in ConnectorRegistry.sources.values():
-metrics += list(db.session.query(datasource_class.metric_class).all())
-
-for metric in metrics:
-if metric.is_restricted:
-merge_pv('metric_access', metric.perm)
-
-
-def clean_perms():
-"""FAB leaves faulty permissions that need to be cleaned up"""
-

[GitHub] timifasubaa commented on a change in pull request #4565: [security] Refactor security code into SupersetSecurityManager

2018-03-26 Thread GitBox
timifasubaa commented on a change in pull request #4565: [security] Refactor 
security code into SupersetSecurityManager
URL: 
https://github.com/apache/incubator-superset/pull/4565#discussion_r177277692
 
 

 ##
 File path: superset/security.py
 ##
 @@ -77,177 +78,303 @@
 ])
 
 
-def merge_perm(sm, permission_name, view_menu_name):
 
 Review comment:
   Will do 


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 a change in pull request #4565: [security] Refactor security code into SupersetSecurityManager

2018-03-26 Thread GitBox
timifasubaa commented on a change in pull request #4565: [security] Refactor 
security code into SupersetSecurityManager
URL: 
https://github.com/apache/incubator-superset/pull/4565#discussion_r177277681
 
 

 ##
 File path: superset/data/__init__.py
 ##
 @@ -71,7 +70,7 @@ def load_energy():
 if not tbl:
 tbl = TBL(table_name=tbl_name)
 tbl.description = "Energy consumption"
-tbl.database = get_or_create_main_db()
+tbl.database = sm.get_or_create_main_db()
 
 Review comment:
   Done


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 a change in pull request #4565: [security] Refactor security code into SupersetSecurityManager

2018-03-26 Thread GitBox
timifasubaa commented on a change in pull request #4565: [security] Refactor 
security code into SupersetSecurityManager
URL: 
https://github.com/apache/incubator-superset/pull/4565#discussion_r177266990
 
 

 ##
 File path: superset/__init__.py
 ##
 @@ -19,7 +19,8 @@
 from werkzeug.contrib.fixers import ProxyFix
 
 from superset.connectors.connector_registry import ConnectorRegistry
-from superset import utils, config  # noqa
+from superset import config, utils # noqa
 
 Review comment:
   Not sure. I'm trying to remove it now. 


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 a change in pull request #4565: [security] Refactor security code into SupersetSecurityManager

2018-03-26 Thread GitBox
timifasubaa commented on a change in pull request #4565: [security] Refactor 
security code into SupersetSecurityManager
URL: 
https://github.com/apache/incubator-superset/pull/4565#discussion_r177267268
 
 

 ##
 File path: superset/__init__.py
 ##
 @@ -149,12 +150,16 @@ def index(self):
 return redirect('/superset/welcome')
 
 
+custom_sm = app.config.get('CUSTOM_SECURITY_MANAGER')
 
 Review comment:
   Good catch. Done!


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 a change in pull request #4565: [security] Refactor security code into SupersetSecurityManager

2018-03-26 Thread GitBox
timifasubaa commented on a change in pull request #4565: [security] Refactor 
security code into SupersetSecurityManager
URL: 
https://github.com/apache/incubator-superset/pull/4565#discussion_r177276966
 
 

 ##
 File path: superset/security.py
 ##
 @@ -77,177 +78,303 @@
 ])
 
 
-def merge_perm(sm, permission_name, view_menu_name):
-# Implementation copied from sm.find_permission_view_menu.
-# TODO: use sm.find_permission_view_menu once issue
-#   https://github.com/airbnb/superset/issues/1944 is resolved.
-permission = sm.find_permission(permission_name)
-view_menu = sm.find_view_menu(view_menu_name)
-pv = None
-if permission and view_menu:
-pv = sm.get_session.query(sm.permissionview_model).filter_by(
-permission=permission, view_menu=view_menu).first()
-if not pv and permission_name and view_menu_name:
-sm.add_permission_view_menu(permission_name, view_menu_name)
-
-
-def is_user_defined_permission(perm):
-return perm.permission.name in OBJECT_SPEC_PERMISSIONS
-
-
-def get_or_create_main_db():
-logging.info('Creating database reference')
-dbobj = (
-db.session.query(models.Database)
-.filter_by(database_name='main')
-.first()
-)
-if not dbobj:
-dbobj = models.Database(database_name='main')
-dbobj.set_sqlalchemy_uri(conf.get('SQLALCHEMY_DATABASE_URI'))
-dbobj.expose_in_sqllab = True
-dbobj.allow_run_sync = True
-db.session.add(dbobj)
-db.session.commit()
-return dbobj
-
-
-def is_admin_only(pvm):
-# not readonly operations on read only model views allowed only for admins
-if (pvm.view_menu.name in READ_ONLY_MODEL_VIEWS and
-pvm.permission.name not in READ_ONLY_PERMISSION):
-return True
-return (
-pvm.view_menu.name in ADMIN_ONLY_VIEW_MENUS or
-pvm.permission.name in ADMIN_ONLY_PERMISSIONS
-)
-
-
-def is_alpha_only(pvm):
-if (pvm.view_menu.name in GAMMA_READ_ONLY_MODEL_VIEWS and
-pvm.permission.name not in READ_ONLY_PERMISSION):
-return True
-return (
-pvm.view_menu.name in ALPHA_ONLY_VIEW_MENUS or
-pvm.permission.name in ALPHA_ONLY_PERMISSIONS
-)
-
-
-def is_admin_pvm(pvm):
-return not is_user_defined_permission(pvm)
-
-
-def is_alpha_pvm(pvm):
-return not (is_user_defined_permission(pvm) or is_admin_only(pvm))
-
-
-def is_gamma_pvm(pvm):
-return not (is_user_defined_permission(pvm) or is_admin_only(pvm) or
-is_alpha_only(pvm))
-
-
-def is_sql_lab_pvm(pvm):
-return pvm.view_menu.name in {'SQL Lab'} or pvm.permission.name in {
-'can_sql_json', 'can_csv', 'can_search_queries',
-}
-
-
-def is_granter_pvm(pvm):
-return pvm.permission.name in {
-'can_override_role_permissions', 'can_approve',
-}
-
-
-def set_role(role_name, pvm_check):
-logging.info('Syncing {} perms'.format(role_name))
-sesh = sm.get_session()
-pvms = sesh.query(ab_models.PermissionView).all()
-pvms = [p for p in pvms if p.permission and p.view_menu]
-role = sm.add_role(role_name)
-role_pvms = [p for p in pvms if pvm_check(p)]
-role.permissions = role_pvms
-sesh.merge(role)
-sesh.commit()
-
-
-def create_custom_permissions():
-# Global perms
-merge_perm(sm, 'all_datasource_access', 'all_datasource_access')
-merge_perm(sm, 'all_database_access', 'all_database_access')
-
-
-def create_missing_perms():
-"""Creates missing perms for datasources, schemas and metrics"""
-
-logging.info(
-'Fetching a set of all perms to lookup which ones are missing')
-all_pvs = set()
-for pv in sm.get_session.query(sm.permissionview_model).all():
-if pv.permission and pv.view_menu:
-all_pvs.add((pv.permission.name, pv.view_menu.name))
-
-def merge_pv(view_menu, perm):
-"""Create permission view menu only if it doesn't exist"""
-if view_menu and perm and (view_menu, perm) not in all_pvs:
-merge_perm(sm, view_menu, perm)
-
-logging.info('Creating missing datasource permissions.')
-datasources = ConnectorRegistry.get_all_datasources(db.session)
-for datasource in datasources:
-merge_pv('datasource_access', datasource.get_perm())
-merge_pv('schema_access', datasource.schema_perm)
-
-logging.info('Creating missing database permissions.')
-databases = db.session.query(models.Database).all()
-for database in databases:
-merge_pv('database_access', database.perm)
-
-logging.info('Creating missing metrics permissions')
-metrics = []
-for datasource_class in ConnectorRegistry.sources.values():
-metrics += list(db.session.query(datasource_class.metric_class).all())
-
-for metric in metrics:
-if metric.is_restricted:
-merge_pv('metric_access', metric.perm)
-
-
-def clean_perms():
-"""FAB leaves faulty permissions that need to be cleaned up"""
-

[GitHub] timifasubaa commented on a change in pull request #4565: [security] Refactor security code into SupersetSecurityManager

2018-03-26 Thread GitBox
timifasubaa commented on a change in pull request #4565: [security] Refactor 
security code into SupersetSecurityManager
URL: 
https://github.com/apache/incubator-superset/pull/4565#discussion_r177268331
 
 

 ##
 File path: superset/data/__init__.py
 ##
 @@ -71,7 +70,7 @@ def load_energy():
 if not tbl:
 tbl = TBL(table_name=tbl_name)
 tbl.description = "Energy consumption"
-tbl.database = get_or_create_main_db()
+tbl.database = sm.get_or_create_main_db()
 
 Review comment:
   Completely agree. I will send a follow up PR to move the non intuitive 
methods to a better place


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 a change in pull request #4565: [security] Refactor security code into SupersetSecurityManager

2018-03-26 Thread GitBox
timifasubaa commented on a change in pull request #4565: [security] Refactor 
security code into SupersetSecurityManager
URL: 
https://github.com/apache/incubator-superset/pull/4565#discussion_r177276922
 
 

 ##
 File path: superset/security.py
 ##
 @@ -77,177 +78,303 @@
 ])
 
 
-def merge_perm(sm, permission_name, view_menu_name):
-# Implementation copied from sm.find_permission_view_menu.
-# TODO: use sm.find_permission_view_menu once issue
-#   https://github.com/airbnb/superset/issues/1944 is resolved.
-permission = sm.find_permission(permission_name)
-view_menu = sm.find_view_menu(view_menu_name)
-pv = None
-if permission and view_menu:
-pv = sm.get_session.query(sm.permissionview_model).filter_by(
-permission=permission, view_menu=view_menu).first()
-if not pv and permission_name and view_menu_name:
-sm.add_permission_view_menu(permission_name, view_menu_name)
-
-
-def is_user_defined_permission(perm):
-return perm.permission.name in OBJECT_SPEC_PERMISSIONS
-
-
-def get_or_create_main_db():
-logging.info('Creating database reference')
-dbobj = (
-db.session.query(models.Database)
-.filter_by(database_name='main')
-.first()
-)
-if not dbobj:
-dbobj = models.Database(database_name='main')
-dbobj.set_sqlalchemy_uri(conf.get('SQLALCHEMY_DATABASE_URI'))
-dbobj.expose_in_sqllab = True
-dbobj.allow_run_sync = True
-db.session.add(dbobj)
-db.session.commit()
-return dbobj
-
-
-def is_admin_only(pvm):
-# not readonly operations on read only model views allowed only for admins
-if (pvm.view_menu.name in READ_ONLY_MODEL_VIEWS and
-pvm.permission.name not in READ_ONLY_PERMISSION):
-return True
-return (
-pvm.view_menu.name in ADMIN_ONLY_VIEW_MENUS or
-pvm.permission.name in ADMIN_ONLY_PERMISSIONS
-)
-
-
-def is_alpha_only(pvm):
-if (pvm.view_menu.name in GAMMA_READ_ONLY_MODEL_VIEWS and
-pvm.permission.name not in READ_ONLY_PERMISSION):
-return True
-return (
-pvm.view_menu.name in ALPHA_ONLY_VIEW_MENUS or
-pvm.permission.name in ALPHA_ONLY_PERMISSIONS
-)
-
-
-def is_admin_pvm(pvm):
-return not is_user_defined_permission(pvm)
-
-
-def is_alpha_pvm(pvm):
-return not (is_user_defined_permission(pvm) or is_admin_only(pvm))
-
-
-def is_gamma_pvm(pvm):
-return not (is_user_defined_permission(pvm) or is_admin_only(pvm) or
-is_alpha_only(pvm))
-
-
-def is_sql_lab_pvm(pvm):
-return pvm.view_menu.name in {'SQL Lab'} or pvm.permission.name in {
-'can_sql_json', 'can_csv', 'can_search_queries',
-}
-
-
-def is_granter_pvm(pvm):
-return pvm.permission.name in {
-'can_override_role_permissions', 'can_approve',
-}
-
-
-def set_role(role_name, pvm_check):
-logging.info('Syncing {} perms'.format(role_name))
-sesh = sm.get_session()
-pvms = sesh.query(ab_models.PermissionView).all()
-pvms = [p for p in pvms if p.permission and p.view_menu]
-role = sm.add_role(role_name)
-role_pvms = [p for p in pvms if pvm_check(p)]
-role.permissions = role_pvms
-sesh.merge(role)
-sesh.commit()
-
-
-def create_custom_permissions():
-# Global perms
-merge_perm(sm, 'all_datasource_access', 'all_datasource_access')
-merge_perm(sm, 'all_database_access', 'all_database_access')
-
-
-def create_missing_perms():
-"""Creates missing perms for datasources, schemas and metrics"""
-
-logging.info(
-'Fetching a set of all perms to lookup which ones are missing')
-all_pvs = set()
-for pv in sm.get_session.query(sm.permissionview_model).all():
-if pv.permission and pv.view_menu:
-all_pvs.add((pv.permission.name, pv.view_menu.name))
-
-def merge_pv(view_menu, perm):
-"""Create permission view menu only if it doesn't exist"""
-if view_menu and perm and (view_menu, perm) not in all_pvs:
-merge_perm(sm, view_menu, perm)
-
-logging.info('Creating missing datasource permissions.')
-datasources = ConnectorRegistry.get_all_datasources(db.session)
-for datasource in datasources:
-merge_pv('datasource_access', datasource.get_perm())
-merge_pv('schema_access', datasource.schema_perm)
-
-logging.info('Creating missing database permissions.')
-databases = db.session.query(models.Database).all()
-for database in databases:
-merge_pv('database_access', database.perm)
-
-logging.info('Creating missing metrics permissions')
-metrics = []
-for datasource_class in ConnectorRegistry.sources.values():
-metrics += list(db.session.query(datasource_class.metric_class).all())
-
-for metric in metrics:
-if metric.is_restricted:
-merge_pv('metric_access', metric.perm)
-
-
-def clean_perms():
-"""FAB leaves faulty permissions that need to be cleaned up"""
-

[GitHub] michellethomas commented on a change in pull request #4556: [WIP] Preview data

2018-03-26 Thread GitBox
michellethomas commented on a change in pull request #4556: [WIP] Preview data
URL: 
https://github.com/apache/incubator-superset/pull/4556#discussion_r177266387
 
 

 ##
 File path: superset/assets/javascripts/explore/components/PreviewChartPanel.jsx
 ##
 @@ -0,0 +1,105 @@
+import React from 'react';
+import PropTypes from 'prop-types';
+import $ from 'jquery';
+import { List } from 'immutable';
+import { Table } from 'reactable';
+import { withTableAutoSizer } from '@data-ui/data-table';
 
 Review comment:
   Chris uses react-virtualized AutoSizer in 
[data-table](https://github.com/williaster/data-ui/blob/master/packages/data-table/src/enhancers/withTableAutoSizer.jsx),
 so you can use react-virtualized to avoid adding another js package.


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] john-bodley commented on a change in pull request #4565: [security] Refactor security code into SupersetSecurityManager

2018-03-26 Thread GitBox
john-bodley commented on a change in pull request #4565: [security] Refactor 
security code into SupersetSecurityManager
URL: 
https://github.com/apache/incubator-superset/pull/4565#discussion_r177265251
 
 

 ##
 File path: superset/security.py
 ##
 @@ -77,177 +78,303 @@
 ])
 
 
-def merge_perm(sm, permission_name, view_menu_name):
-# Implementation copied from sm.find_permission_view_menu.
-# TODO: use sm.find_permission_view_menu once issue
-#   https://github.com/airbnb/superset/issues/1944 is resolved.
-permission = sm.find_permission(permission_name)
-view_menu = sm.find_view_menu(view_menu_name)
-pv = None
-if permission and view_menu:
-pv = sm.get_session.query(sm.permissionview_model).filter_by(
-permission=permission, view_menu=view_menu).first()
-if not pv and permission_name and view_menu_name:
-sm.add_permission_view_menu(permission_name, view_menu_name)
-
-
-def is_user_defined_permission(perm):
-return perm.permission.name in OBJECT_SPEC_PERMISSIONS
-
-
-def get_or_create_main_db():
-logging.info('Creating database reference')
-dbobj = (
-db.session.query(models.Database)
-.filter_by(database_name='main')
-.first()
-)
-if not dbobj:
-dbobj = models.Database(database_name='main')
-dbobj.set_sqlalchemy_uri(conf.get('SQLALCHEMY_DATABASE_URI'))
-dbobj.expose_in_sqllab = True
-dbobj.allow_run_sync = True
-db.session.add(dbobj)
-db.session.commit()
-return dbobj
-
-
-def is_admin_only(pvm):
-# not readonly operations on read only model views allowed only for admins
-if (pvm.view_menu.name in READ_ONLY_MODEL_VIEWS and
-pvm.permission.name not in READ_ONLY_PERMISSION):
-return True
-return (
-pvm.view_menu.name in ADMIN_ONLY_VIEW_MENUS or
-pvm.permission.name in ADMIN_ONLY_PERMISSIONS
-)
-
-
-def is_alpha_only(pvm):
-if (pvm.view_menu.name in GAMMA_READ_ONLY_MODEL_VIEWS and
-pvm.permission.name not in READ_ONLY_PERMISSION):
-return True
-return (
-pvm.view_menu.name in ALPHA_ONLY_VIEW_MENUS or
-pvm.permission.name in ALPHA_ONLY_PERMISSIONS
-)
-
-
-def is_admin_pvm(pvm):
-return not is_user_defined_permission(pvm)
-
-
-def is_alpha_pvm(pvm):
-return not (is_user_defined_permission(pvm) or is_admin_only(pvm))
-
-
-def is_gamma_pvm(pvm):
-return not (is_user_defined_permission(pvm) or is_admin_only(pvm) or
-is_alpha_only(pvm))
-
-
-def is_sql_lab_pvm(pvm):
-return pvm.view_menu.name in {'SQL Lab'} or pvm.permission.name in {
-'can_sql_json', 'can_csv', 'can_search_queries',
-}
-
-
-def is_granter_pvm(pvm):
-return pvm.permission.name in {
-'can_override_role_permissions', 'can_approve',
-}
-
-
-def set_role(role_name, pvm_check):
-logging.info('Syncing {} perms'.format(role_name))
-sesh = sm.get_session()
-pvms = sesh.query(ab_models.PermissionView).all()
-pvms = [p for p in pvms if p.permission and p.view_menu]
-role = sm.add_role(role_name)
-role_pvms = [p for p in pvms if pvm_check(p)]
-role.permissions = role_pvms
-sesh.merge(role)
-sesh.commit()
-
-
-def create_custom_permissions():
-# Global perms
-merge_perm(sm, 'all_datasource_access', 'all_datasource_access')
-merge_perm(sm, 'all_database_access', 'all_database_access')
-
-
-def create_missing_perms():
-"""Creates missing perms for datasources, schemas and metrics"""
-
-logging.info(
-'Fetching a set of all perms to lookup which ones are missing')
-all_pvs = set()
-for pv in sm.get_session.query(sm.permissionview_model).all():
-if pv.permission and pv.view_menu:
-all_pvs.add((pv.permission.name, pv.view_menu.name))
-
-def merge_pv(view_menu, perm):
-"""Create permission view menu only if it doesn't exist"""
-if view_menu and perm and (view_menu, perm) not in all_pvs:
-merge_perm(sm, view_menu, perm)
-
-logging.info('Creating missing datasource permissions.')
-datasources = ConnectorRegistry.get_all_datasources(db.session)
-for datasource in datasources:
-merge_pv('datasource_access', datasource.get_perm())
-merge_pv('schema_access', datasource.schema_perm)
-
-logging.info('Creating missing database permissions.')
-databases = db.session.query(models.Database).all()
-for database in databases:
-merge_pv('database_access', database.perm)
-
-logging.info('Creating missing metrics permissions')
-metrics = []
-for datasource_class in ConnectorRegistry.sources.values():
-metrics += list(db.session.query(datasource_class.metric_class).all())
-
-for metric in metrics:
-if metric.is_restricted:
-merge_pv('metric_access', metric.perm)
-
-
-def clean_perms():
-"""FAB leaves faulty permissions that need to be cleaned up"""
-

[GitHub] michellethomas commented on issue #4649: Use one library for table components

2018-03-26 Thread GitBox
michellethomas commented on issue #4649: Use one library for table components
URL: 
https://github.com/apache/incubator-superset/issues/4649#issuecomment-376344407
 
 
   After talking with Chris a bit more about this, for now I think we are just 
going to use `react-virtualized` until there's a clear need to use something 
within `data-ui/data-table`.
   
   I'm going to start by moving the Table viz over to react-virtualized.


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] john-bodley commented on a change in pull request #4565: [security] Refactor security code into SupersetSecurityManager

2018-03-26 Thread GitBox
john-bodley commented on a change in pull request #4565: [security] Refactor 
security code into SupersetSecurityManager
URL: 
https://github.com/apache/incubator-superset/pull/4565#discussion_r177264873
 
 

 ##
 File path: superset/security.py
 ##
 @@ -77,177 +78,303 @@
 ])
 
 
-def merge_perm(sm, permission_name, view_menu_name):
-# Implementation copied from sm.find_permission_view_menu.
-# TODO: use sm.find_permission_view_menu once issue
-#   https://github.com/airbnb/superset/issues/1944 is resolved.
-permission = sm.find_permission(permission_name)
-view_menu = sm.find_view_menu(view_menu_name)
-pv = None
-if permission and view_menu:
-pv = sm.get_session.query(sm.permissionview_model).filter_by(
-permission=permission, view_menu=view_menu).first()
-if not pv and permission_name and view_menu_name:
-sm.add_permission_view_menu(permission_name, view_menu_name)
-
-
-def is_user_defined_permission(perm):
-return perm.permission.name in OBJECT_SPEC_PERMISSIONS
-
-
-def get_or_create_main_db():
-logging.info('Creating database reference')
-dbobj = (
-db.session.query(models.Database)
-.filter_by(database_name='main')
-.first()
-)
-if not dbobj:
-dbobj = models.Database(database_name='main')
-dbobj.set_sqlalchemy_uri(conf.get('SQLALCHEMY_DATABASE_URI'))
-dbobj.expose_in_sqllab = True
-dbobj.allow_run_sync = True
-db.session.add(dbobj)
-db.session.commit()
-return dbobj
-
-
-def is_admin_only(pvm):
-# not readonly operations on read only model views allowed only for admins
-if (pvm.view_menu.name in READ_ONLY_MODEL_VIEWS and
-pvm.permission.name not in READ_ONLY_PERMISSION):
-return True
-return (
-pvm.view_menu.name in ADMIN_ONLY_VIEW_MENUS or
-pvm.permission.name in ADMIN_ONLY_PERMISSIONS
-)
-
-
-def is_alpha_only(pvm):
-if (pvm.view_menu.name in GAMMA_READ_ONLY_MODEL_VIEWS and
-pvm.permission.name not in READ_ONLY_PERMISSION):
-return True
-return (
-pvm.view_menu.name in ALPHA_ONLY_VIEW_MENUS or
-pvm.permission.name in ALPHA_ONLY_PERMISSIONS
-)
-
-
-def is_admin_pvm(pvm):
-return not is_user_defined_permission(pvm)
-
-
-def is_alpha_pvm(pvm):
-return not (is_user_defined_permission(pvm) or is_admin_only(pvm))
-
-
-def is_gamma_pvm(pvm):
-return not (is_user_defined_permission(pvm) or is_admin_only(pvm) or
-is_alpha_only(pvm))
-
-
-def is_sql_lab_pvm(pvm):
-return pvm.view_menu.name in {'SQL Lab'} or pvm.permission.name in {
-'can_sql_json', 'can_csv', 'can_search_queries',
-}
-
-
-def is_granter_pvm(pvm):
-return pvm.permission.name in {
-'can_override_role_permissions', 'can_approve',
-}
-
-
-def set_role(role_name, pvm_check):
-logging.info('Syncing {} perms'.format(role_name))
-sesh = sm.get_session()
-pvms = sesh.query(ab_models.PermissionView).all()
-pvms = [p for p in pvms if p.permission and p.view_menu]
-role = sm.add_role(role_name)
-role_pvms = [p for p in pvms if pvm_check(p)]
-role.permissions = role_pvms
-sesh.merge(role)
-sesh.commit()
-
-
-def create_custom_permissions():
-# Global perms
-merge_perm(sm, 'all_datasource_access', 'all_datasource_access')
-merge_perm(sm, 'all_database_access', 'all_database_access')
-
-
-def create_missing_perms():
-"""Creates missing perms for datasources, schemas and metrics"""
-
-logging.info(
-'Fetching a set of all perms to lookup which ones are missing')
-all_pvs = set()
-for pv in sm.get_session.query(sm.permissionview_model).all():
-if pv.permission and pv.view_menu:
-all_pvs.add((pv.permission.name, pv.view_menu.name))
-
-def merge_pv(view_menu, perm):
-"""Create permission view menu only if it doesn't exist"""
-if view_menu and perm and (view_menu, perm) not in all_pvs:
-merge_perm(sm, view_menu, perm)
-
-logging.info('Creating missing datasource permissions.')
-datasources = ConnectorRegistry.get_all_datasources(db.session)
-for datasource in datasources:
-merge_pv('datasource_access', datasource.get_perm())
-merge_pv('schema_access', datasource.schema_perm)
-
-logging.info('Creating missing database permissions.')
-databases = db.session.query(models.Database).all()
-for database in databases:
-merge_pv('database_access', database.perm)
-
-logging.info('Creating missing metrics permissions')
-metrics = []
-for datasource_class in ConnectorRegistry.sources.values():
-metrics += list(db.session.query(datasource_class.metric_class).all())
-
-for metric in metrics:
-if metric.is_restricted:
-merge_pv('metric_access', metric.perm)
-
-
-def clean_perms():
-"""FAB leaves faulty permissions that need to be cleaned up"""
-

[GitHub] john-bodley commented on a change in pull request #4565: [security] Refactor security code into SupersetSecurityManager

2018-03-26 Thread GitBox
john-bodley commented on a change in pull request #4565: [security] Refactor 
security code into SupersetSecurityManager
URL: 
https://github.com/apache/incubator-superset/pull/4565#discussion_r177264873
 
 

 ##
 File path: superset/security.py
 ##
 @@ -77,177 +78,303 @@
 ])
 
 
-def merge_perm(sm, permission_name, view_menu_name):
-# Implementation copied from sm.find_permission_view_menu.
-# TODO: use sm.find_permission_view_menu once issue
-#   https://github.com/airbnb/superset/issues/1944 is resolved.
-permission = sm.find_permission(permission_name)
-view_menu = sm.find_view_menu(view_menu_name)
-pv = None
-if permission and view_menu:
-pv = sm.get_session.query(sm.permissionview_model).filter_by(
-permission=permission, view_menu=view_menu).first()
-if not pv and permission_name and view_menu_name:
-sm.add_permission_view_menu(permission_name, view_menu_name)
-
-
-def is_user_defined_permission(perm):
-return perm.permission.name in OBJECT_SPEC_PERMISSIONS
-
-
-def get_or_create_main_db():
-logging.info('Creating database reference')
-dbobj = (
-db.session.query(models.Database)
-.filter_by(database_name='main')
-.first()
-)
-if not dbobj:
-dbobj = models.Database(database_name='main')
-dbobj.set_sqlalchemy_uri(conf.get('SQLALCHEMY_DATABASE_URI'))
-dbobj.expose_in_sqllab = True
-dbobj.allow_run_sync = True
-db.session.add(dbobj)
-db.session.commit()
-return dbobj
-
-
-def is_admin_only(pvm):
-# not readonly operations on read only model views allowed only for admins
-if (pvm.view_menu.name in READ_ONLY_MODEL_VIEWS and
-pvm.permission.name not in READ_ONLY_PERMISSION):
-return True
-return (
-pvm.view_menu.name in ADMIN_ONLY_VIEW_MENUS or
-pvm.permission.name in ADMIN_ONLY_PERMISSIONS
-)
-
-
-def is_alpha_only(pvm):
-if (pvm.view_menu.name in GAMMA_READ_ONLY_MODEL_VIEWS and
-pvm.permission.name not in READ_ONLY_PERMISSION):
-return True
-return (
-pvm.view_menu.name in ALPHA_ONLY_VIEW_MENUS or
-pvm.permission.name in ALPHA_ONLY_PERMISSIONS
-)
-
-
-def is_admin_pvm(pvm):
-return not is_user_defined_permission(pvm)
-
-
-def is_alpha_pvm(pvm):
-return not (is_user_defined_permission(pvm) or is_admin_only(pvm))
-
-
-def is_gamma_pvm(pvm):
-return not (is_user_defined_permission(pvm) or is_admin_only(pvm) or
-is_alpha_only(pvm))
-
-
-def is_sql_lab_pvm(pvm):
-return pvm.view_menu.name in {'SQL Lab'} or pvm.permission.name in {
-'can_sql_json', 'can_csv', 'can_search_queries',
-}
-
-
-def is_granter_pvm(pvm):
-return pvm.permission.name in {
-'can_override_role_permissions', 'can_approve',
-}
-
-
-def set_role(role_name, pvm_check):
-logging.info('Syncing {} perms'.format(role_name))
-sesh = sm.get_session()
-pvms = sesh.query(ab_models.PermissionView).all()
-pvms = [p for p in pvms if p.permission and p.view_menu]
-role = sm.add_role(role_name)
-role_pvms = [p for p in pvms if pvm_check(p)]
-role.permissions = role_pvms
-sesh.merge(role)
-sesh.commit()
-
-
-def create_custom_permissions():
-# Global perms
-merge_perm(sm, 'all_datasource_access', 'all_datasource_access')
-merge_perm(sm, 'all_database_access', 'all_database_access')
-
-
-def create_missing_perms():
-"""Creates missing perms for datasources, schemas and metrics"""
-
-logging.info(
-'Fetching a set of all perms to lookup which ones are missing')
-all_pvs = set()
-for pv in sm.get_session.query(sm.permissionview_model).all():
-if pv.permission and pv.view_menu:
-all_pvs.add((pv.permission.name, pv.view_menu.name))
-
-def merge_pv(view_menu, perm):
-"""Create permission view menu only if it doesn't exist"""
-if view_menu and perm and (view_menu, perm) not in all_pvs:
-merge_perm(sm, view_menu, perm)
-
-logging.info('Creating missing datasource permissions.')
-datasources = ConnectorRegistry.get_all_datasources(db.session)
-for datasource in datasources:
-merge_pv('datasource_access', datasource.get_perm())
-merge_pv('schema_access', datasource.schema_perm)
-
-logging.info('Creating missing database permissions.')
-databases = db.session.query(models.Database).all()
-for database in databases:
-merge_pv('database_access', database.perm)
-
-logging.info('Creating missing metrics permissions')
-metrics = []
-for datasource_class in ConnectorRegistry.sources.values():
-metrics += list(db.session.query(datasource_class.metric_class).all())
-
-for metric in metrics:
-if metric.is_restricted:
-merge_pv('metric_access', metric.perm)
-
-
-def clean_perms():
-"""FAB leaves faulty permissions that need to be cleaned up"""
-

[GitHub] john-bodley commented on a change in pull request #4565: [security] Refactor security code into SupersetSecurityManager

2018-03-26 Thread GitBox
john-bodley commented on a change in pull request #4565: [security] Refactor 
security code into SupersetSecurityManager
URL: 
https://github.com/apache/incubator-superset/pull/4565#discussion_r177264289
 
 

 ##
 File path: superset/security.py
 ##
 @@ -77,177 +78,303 @@
 ])
 
 
-def merge_perm(sm, permission_name, view_menu_name):
-# Implementation copied from sm.find_permission_view_menu.
-# TODO: use sm.find_permission_view_menu once issue
-#   https://github.com/airbnb/superset/issues/1944 is resolved.
-permission = sm.find_permission(permission_name)
-view_menu = sm.find_view_menu(view_menu_name)
-pv = None
-if permission and view_menu:
-pv = sm.get_session.query(sm.permissionview_model).filter_by(
-permission=permission, view_menu=view_menu).first()
-if not pv and permission_name and view_menu_name:
-sm.add_permission_view_menu(permission_name, view_menu_name)
-
-
-def is_user_defined_permission(perm):
-return perm.permission.name in OBJECT_SPEC_PERMISSIONS
-
-
-def get_or_create_main_db():
-logging.info('Creating database reference')
-dbobj = (
-db.session.query(models.Database)
-.filter_by(database_name='main')
-.first()
-)
-if not dbobj:
-dbobj = models.Database(database_name='main')
-dbobj.set_sqlalchemy_uri(conf.get('SQLALCHEMY_DATABASE_URI'))
-dbobj.expose_in_sqllab = True
-dbobj.allow_run_sync = True
-db.session.add(dbobj)
-db.session.commit()
-return dbobj
-
-
-def is_admin_only(pvm):
-# not readonly operations on read only model views allowed only for admins
-if (pvm.view_menu.name in READ_ONLY_MODEL_VIEWS and
-pvm.permission.name not in READ_ONLY_PERMISSION):
-return True
-return (
-pvm.view_menu.name in ADMIN_ONLY_VIEW_MENUS or
-pvm.permission.name in ADMIN_ONLY_PERMISSIONS
-)
-
-
-def is_alpha_only(pvm):
-if (pvm.view_menu.name in GAMMA_READ_ONLY_MODEL_VIEWS and
-pvm.permission.name not in READ_ONLY_PERMISSION):
-return True
-return (
-pvm.view_menu.name in ALPHA_ONLY_VIEW_MENUS or
-pvm.permission.name in ALPHA_ONLY_PERMISSIONS
-)
-
-
-def is_admin_pvm(pvm):
-return not is_user_defined_permission(pvm)
-
-
-def is_alpha_pvm(pvm):
-return not (is_user_defined_permission(pvm) or is_admin_only(pvm))
-
-
-def is_gamma_pvm(pvm):
-return not (is_user_defined_permission(pvm) or is_admin_only(pvm) or
-is_alpha_only(pvm))
-
-
-def is_sql_lab_pvm(pvm):
-return pvm.view_menu.name in {'SQL Lab'} or pvm.permission.name in {
-'can_sql_json', 'can_csv', 'can_search_queries',
-}
-
-
-def is_granter_pvm(pvm):
-return pvm.permission.name in {
-'can_override_role_permissions', 'can_approve',
-}
-
-
-def set_role(role_name, pvm_check):
-logging.info('Syncing {} perms'.format(role_name))
-sesh = sm.get_session()
-pvms = sesh.query(ab_models.PermissionView).all()
-pvms = [p for p in pvms if p.permission and p.view_menu]
-role = sm.add_role(role_name)
-role_pvms = [p for p in pvms if pvm_check(p)]
-role.permissions = role_pvms
-sesh.merge(role)
-sesh.commit()
-
-
-def create_custom_permissions():
-# Global perms
-merge_perm(sm, 'all_datasource_access', 'all_datasource_access')
-merge_perm(sm, 'all_database_access', 'all_database_access')
-
-
-def create_missing_perms():
-"""Creates missing perms for datasources, schemas and metrics"""
-
-logging.info(
-'Fetching a set of all perms to lookup which ones are missing')
-all_pvs = set()
-for pv in sm.get_session.query(sm.permissionview_model).all():
-if pv.permission and pv.view_menu:
-all_pvs.add((pv.permission.name, pv.view_menu.name))
-
-def merge_pv(view_menu, perm):
-"""Create permission view menu only if it doesn't exist"""
-if view_menu and perm and (view_menu, perm) not in all_pvs:
-merge_perm(sm, view_menu, perm)
-
-logging.info('Creating missing datasource permissions.')
-datasources = ConnectorRegistry.get_all_datasources(db.session)
-for datasource in datasources:
-merge_pv('datasource_access', datasource.get_perm())
-merge_pv('schema_access', datasource.schema_perm)
-
-logging.info('Creating missing database permissions.')
-databases = db.session.query(models.Database).all()
-for database in databases:
-merge_pv('database_access', database.perm)
-
-logging.info('Creating missing metrics permissions')
-metrics = []
-for datasource_class in ConnectorRegistry.sources.values():
-metrics += list(db.session.query(datasource_class.metric_class).all())
-
-for metric in metrics:
-if metric.is_restricted:
-merge_pv('metric_access', metric.perm)
-
-
-def clean_perms():
-"""FAB leaves faulty permissions that need to be cleaned up"""
-

[GitHub] john-bodley commented on a change in pull request #4565: [security] Refactor security code into SupersetSecurityManager

2018-03-26 Thread GitBox
john-bodley commented on a change in pull request #4565: [security] Refactor 
security code into SupersetSecurityManager
URL: 
https://github.com/apache/incubator-superset/pull/4565#discussion_r177262463
 
 

 ##
 File path: superset/security.py
 ##
 @@ -77,177 +78,303 @@
 ])
 
 
-def merge_perm(sm, permission_name, view_menu_name):
 
 Review comment:
   It probably merits exampling in the PR description what's going here, i.e., 
the `SupersetSecurityManager` class in a combination of the previously defined 
methods in `security.py` in combination with accessor methods previously 
defined in the `BaseSupersetView` class which aren't view specific 
(_hypothesizing_). 


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] john-bodley commented on a change in pull request #4565: [security] Refactor security code into SupersetSecurityManager

2018-03-26 Thread GitBox
john-bodley commented on a change in pull request #4565: [security] Refactor 
security code into SupersetSecurityManager
URL: 
https://github.com/apache/incubator-superset/pull/4565#discussion_r177260356
 
 

 ##
 File path: superset/__init__.py
 ##
 @@ -149,12 +150,16 @@ def index(self):
 return redirect('/superset/welcome')
 
 
+custom_sm = app.config.get('CUSTOM_SECURITY_MANAGER')
 
 Review comment:
   This can be shortened to be:
   ```
   sm = app.config.get('CUSTOM_SECURITY_MANAGER', SupersetSecurityManager)
   ```


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] john-bodley commented on a change in pull request #4565: [security] Refactor security code into SupersetSecurityManager

2018-03-26 Thread GitBox
john-bodley commented on a change in pull request #4565: [security] Refactor 
security code into SupersetSecurityManager
URL: 
https://github.com/apache/incubator-superset/pull/4565#discussion_r177260067
 
 

 ##
 File path: superset/__init__.py
 ##
 @@ -19,7 +19,8 @@
 from werkzeug.contrib.fixers import ProxyFix
 
 from superset.connectors.connector_registry import ConnectorRegistry
-from superset import utils, config  # noqa
+from superset import config, utils # noqa
 
 Review comment:
   Do you know why the `# noqa` is present 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] john-bodley commented on a change in pull request #4565: [security] Refactor security code into SupersetSecurityManager

2018-03-26 Thread GitBox
john-bodley commented on a change in pull request #4565: [security] Refactor 
security code into SupersetSecurityManager
URL: 
https://github.com/apache/incubator-superset/pull/4565#discussion_r177261765
 
 

 ##
 File path: superset/data/__init__.py
 ##
 @@ -71,7 +70,7 @@ def load_energy():
 if not tbl:
 tbl = TBL(table_name=tbl_name)
 tbl.description = "Energy consumption"
-tbl.database = get_or_create_main_db()
+tbl.database = sm.get_or_create_main_db()
 
 Review comment:
   Note this somewhat pre-dates this PR, but I'm not certain that 
`get_or_create_main_db` is even security related and thus having the security 
manager get or create the `main` database seems somewhat non-intuitive to me.


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] jdnurmi commented on issue #4690: css not loading at "http://0.0.0.0:8088/superset/welcome"

2018-03-26 Thread GitBox
jdnurmi commented on issue #4690: css not loading at 
"http://0.0.0.0:8088/superset/welcome; 
URL: 
https://github.com/apache/incubator-superset/issues/4690#issuecomment-376326328
 
 
   Sorry, to be clear, there isn't a 0.24.0 on pypi yet - this is using the 
version in the github releases directly via pip install that looks to have 
shipped late last week.


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] xrmx commented on issue #4690: css not loading at "http://0.0.0.0:8088/superset/welcome"

2018-03-26 Thread GitBox
xrmx commented on issue #4690: css not loading at 
"http://0.0.0.0:8088/superset/welcome; 
URL: 
https://github.com/apache/incubator-superset/issues/4690#issuecomment-376322326
 
 
   @jdnurmi That would mean that the package uploaded to pypi is broken.


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] xrmx commented on issue #4690: css not loading at "http://0.0.0.0:8088/superset/welcome"

2018-03-26 Thread GitBox
xrmx commented on issue #4690: css not loading at 
"http://0.0.0.0:8088/superset/welcome; 
URL: 
https://github.com/apache/incubator-superset/issues/4690#issuecomment-376322326
 
 
   @jdnurmi That would mean that the packaged uploaded to pypi is broken.


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] jdnurmi commented on issue #4690: css not loading at "http://0.0.0.0:8088/superset/welcome"

2018-03-26 Thread GitBox
jdnurmi commented on issue #4690: css not loading at 
"http://0.0.0.0:8088/superset/welcome; 
URL: 
https://github.com/apache/incubator-superset/issues/4690#issuecomment-376317604
 
 
   Approximate patch, will likely need fuzzy to apply
   
   ```
   diff --git a/docker/superset/Dockerfile b/docker/superset/Dockerfile
   index d0d1e0a..b573aa4 100644
   --- a/docker/superset/Dockerfile
   +++ b/docker/superset/Dockerfile
   @@ -3,3 +3,3 @@ FROM ...
# Superset version
   -ARG SUPERSET_VERSION=0.23.3
   +ARG SUPERSET_VERSION=0.24.0

   @@ -49,3 +49,3 @@ RUN useradd -U -m superset && \
Werkzeug==0.12.1 \
   -superset==${SUPERSET_VERSION}
   +
https://github.com/apache/incubator-superset/archive/${SUPERSET_VERSION}.tar.gz
   ```


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] jdnurmi commented on issue #4690: css not loading at "http://0.0.0.0:8088/superset/welcome"

2018-03-26 Thread GitBox
jdnurmi commented on issue #4690: css not loading at 
"http://0.0.0.0:8088/superset/welcome; 
URL: 
https://github.com/apache/incubator-superset/issues/4690#issuecomment-376317604
 
 
   Approximate patch, will likely need fuzzy to apply
   
   `
   diff --git a/docker/superset/Dockerfile b/docker/superset/Dockerfile
   index d0d1e0a..b573aa4 100644
   --- a/docker/superset/Dockerfile
   +++ b/docker/superset/Dockerfile
   @@ -3,3 +3,3 @@ FROM registry.internal.sano.co/sano/sano-python3:latest
# Superset version
   -ARG SUPERSET_VERSION=0.23.3
   +ARG SUPERSET_VERSION=0.24.0

   @@ -49,3 +49,3 @@ RUN useradd -U -m superset && \
Werkzeug==0.12.1 \
   -superset==${SUPERSET_VERSION}
   +
https://github.com/apache/incubator-superset/archive/${SUPERSET_VERSION}.tar.gz
   `


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] jdnurmi commented on issue #4690: css not loading at "http://0.0.0.0:8088/superset/welcome"

2018-03-26 Thread GitBox
jdnurmi commented on issue #4690: css not loading at 
"http://0.0.0.0:8088/superset/welcome; 
URL: 
https://github.com/apache/incubator-superset/issues/4690#issuecomment-376313841
 
 
   Not reporter, but similar experience, using the community docker image as a 
base, but shifting to version 0.24.0 (direct pull from github releases as it 
wasn't available on pypi this morning)
   
   HTML Fails to pull a reference in the header:
   `
   ` 
   
   Which to source is 
   `  
   `
   in templates/superset/basic.html.
   
   I started to try to trace js_manifest, but couldn't find a generated 
manifest that it was trying to read, or how to generate one, so I started 
checking in the issues list here on github, and here I am  
   
   


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] jdnurmi commented on issue #4690: css not loading at "http://0.0.0.0:8088/superset/welcome"

2018-03-26 Thread GitBox
jdnurmi commented on issue #4690: css not loading at 
"http://0.0.0.0:8088/superset/welcome; 
URL: 
https://github.com/apache/incubator-superset/issues/4690#issuecomment-376313841
 
 
   Not reporter, but similar experience, using the community docker image as a 
base, but shifting to version 0.24.0 (direct pull from github releases as it 
wasn't available on pypi this morning)
   
   HTML Fails to pull a reference in the header:
   `
   ` 
   
   Which to source is 
 
   
   in templates/superset/basic.html.
   
   I started to try to trace js_manifest, but couldn't find a generated 
manifest that it was trying to read, or how to generate one, so I started 
checking in the issues list here on github, and here I am  
   
   


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] hughhhh commented on a change in pull request #4680: Hotkeys in SQL Lab

2018-03-26 Thread GitBox
hug commented on a change in pull request #4680: Hotkeys in SQL Lab
URL: 
https://github.com/apache/incubator-superset/pull/4680#discussion_r177208754
 
 

 ##
 File path: superset/assets/javascripts/SqlLab/components/SqlEditor.jsx
 ##
 @@ -226,6 +229,32 @@ class SqlEditor extends React.PureComponent {
   render() {
 const height = this.sqlEditorHeight();
 const defaultNorthHeight = this.props.queryEditor.height || 200;
+const SQLLAB_HOTKEYS = [
 
 Review comment:
   Ahhh true, the `func` param, i just don't like the bulky render view, but if 
there isn't a way to clean it up we can just go with 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] mistercrunch commented on a change in pull request #4680: Hotkeys in SQL Lab

2018-03-26 Thread GitBox
mistercrunch commented on a change in pull request #4680: Hotkeys in SQL Lab
URL: 
https://github.com/apache/incubator-superset/pull/4680#discussion_r177197034
 
 

 ##
 File path: superset/assets/javascripts/SqlLab/components/SqlEditor.jsx
 ##
 @@ -226,6 +229,32 @@ class SqlEditor extends React.PureComponent {
   render() {
 const height = this.sqlEditorHeight();
 const defaultNorthHeight = this.props.queryEditor.height || 200;
+const SQLLAB_HOTKEYS = [
 
 Review comment:
   the actions aren't in scope at that point though...


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 #4693: Use 3 letters month prefix in default date format

2018-03-26 Thread GitBox
mistercrunch opened a new pull request #4693: Use 3 letters month prefix in 
default date format
URL: https://github.com/apache/incubator-superset/pull/4693
 
 
   Using 3-letters month name is clear enough and allows for a smaller and even 
x axis margin on line charts.


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 #4651: [WiP] [explore] proper filtering of NULLs and ''

2018-03-26 Thread GitBox
codecov-io commented on issue #4651: [WiP] [explore] proper filtering of NULLs 
and ''
URL: 
https://github.com/apache/incubator-superset/pull/4651#issuecomment-374685460
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4651?src=pr=h1)
 Report
   > Merging 
[#4651](https://codecov.io/gh/apache/incubator-superset/pull/4651?src=pr=desc)
 into 
[master](https://codecov.io/gh/apache/incubator-superset/commit/52b925fee80c0d46aeb444326ac499296d682396?src=pr=desc)
 will **decrease** coverage by `0.14%`.
   > The diff coverage is `76%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-superset/pull/4651/graphs/tree.svg?height=150=650=KsB0fHcx6l=pr)](https://codecov.io/gh/apache/incubator-superset/pull/4651?src=pr=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master#4651  +/-   ##
   ==
   - Coverage71.4%   71.25%   -0.15% 
   ==
 Files 190  190  
 Lines   1493414937   +3 
 Branches 1102 1106   +4 
   ==
   - Hits1066410644  -20 
   - Misses   4267 4290  +23 
 Partials33
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-superset/pull/4651?src=pr=tree)
 | Coverage Δ | |
   |---|---|---|
   | 
[.../assets/javascripts/components/AlteredSliceTag.jsx](https://codecov.io/gh/apache/incubator-superset/pull/4651/diff?src=pr=tree#diff-c3VwZXJzZXQvYXNzZXRzL2phdmFzY3JpcHRzL2NvbXBvbmVudHMvQWx0ZXJlZFNsaWNlVGFnLmpzeA==)
 | `100% <100%> (ø)` | :arrow_up: |
   | 
[...ipts/explore/components/controls/FilterControl.jsx](https://codecov.io/gh/apache/incubator-superset/pull/4651/diff?src=pr=tree#diff-c3VwZXJzZXQvYXNzZXRzL2phdmFzY3JpcHRzL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaWx0ZXJDb250cm9sLmpzeA==)
 | `97.7% <100%> (+0.4%)` | :arrow_up: |
   | 
[superset/connectors/sqla/models.py](https://codecov.io/gh/apache/incubator-superset/pull/4651/diff?src=pr=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==)
 | `77.14% <37.5%> (-0.78%)` | :arrow_down: |
   | 
[...javascripts/explore/components/controls/Filter.jsx](https://codecov.io/gh/apache/incubator-superset/pull/4651/diff?src=pr=tree#diff-c3VwZXJzZXQvYXNzZXRzL2phdmFzY3JpcHRzL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaWx0ZXIuanN4)
 | `78.46% <50%> (-0.91%)` | :arrow_down: |
   | 
[superset/viz.py](https://codecov.io/gh/apache/incubator-superset/pull/4651/diff?src=pr=tree#diff-c3VwZXJzZXQvdml6LnB5)
 | `78.46% <0%> (-1.16%)` | :arrow_down: |
   | 
[superset/assets/javascripts/modules/colors.js](https://codecov.io/gh/apache/incubator-superset/pull/4651/diff?src=pr=tree#diff-c3VwZXJzZXQvYXNzZXRzL2phdmFzY3JpcHRzL21vZHVsZXMvY29sb3JzLmpz)
 | `76.59% <0%> (-0.49%)` | :arrow_down: |
   | 
[superset/sql\_lab.py](https://codecov.io/gh/apache/incubator-superset/pull/4651/diff?src=pr=tree#diff-c3VwZXJzZXQvc3FsX2xhYi5weQ==)
 | `74.21% <0%> (-0.4%)` | :arrow_down: |
   | 
[superset/utils.py](https://codecov.io/gh/apache/incubator-superset/pull/4651/diff?src=pr=tree#diff-c3VwZXJzZXQvdXRpbHMucHk=)
 | `87.74% <0%> (-0.22%)` | :arrow_down: |
   | 
[superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/4651/diff?src=pr=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==)
 | `71.21% <0%> (+0.02%)` | :arrow_up: |
   | 
[superset/forms.py](https://codecov.io/gh/apache/incubator-superset/pull/4651/diff?src=pr=tree#diff-c3VwZXJzZXQvZm9ybXMucHk=)
 | `100% <0%> (+1.88%)` | :arrow_up: |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4651?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/4651?src=pr=footer).
 Last update 
[52b925f...dd6381b](https://codecov.io/gh/apache/incubator-superset/pull/4651?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] graceguo-supercat closed pull request #4683: Fix bug with sorting columns in group by using time shift

2018-03-26 Thread GitBox
graceguo-supercat closed pull request #4683: Fix bug with sorting columns in 
group by using time shift
URL: https://github.com/apache/incubator-superset/pull/4683
 
 
   

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/viz.py b/superset/viz.py
index 6f17ad71a7..fc87430161 100644
--- a/superset/viz.py
+++ b/superset/viz.py
@@ -1149,7 +1149,7 @@ def get_data(self, df):
 
 if self._extra_chart_data:
 chart_data += self._extra_chart_data
-chart_data = sorted(chart_data, key=lambda x: x['key'])
+chart_data = sorted(chart_data, key=lambda x: tuple(x['key']))
 
 return chart_data
 


 


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] john-bodley commented on issue #4669: [sqllab] Using app context for processing Jinja template in async mode

2018-03-26 Thread GitBox
john-bodley commented on issue #4669: [sqllab] Using app context for processing 
Jinja template in async mode
URL: 
https://github.com/apache/incubator-superset/pull/4669#issuecomment-376238469
 
 
   @mistercrunch even though in `sql_lab.py` 
[get_sqla_engine](https://github.com/apache/incubator-superset/blob/master/superset/sql_lab.py#L209)
 is called with the `user_name` parameter, the Jinja template issue we're 
seeing is when one is using `presto.latest_partition` which doesn't pass 
through the username, and from reading through the code this may neither be 
feasible nor non-trivial.
   
   How do you feel about the change? Note I'm somewhat perplex how other 
portions of `execute_sql` work on a Celery worker like 
[get_session](https://github.com/apache/incubator-superset/blob/master/superset/sql_lab.py#L81)
 which references `app` as I presume that would also be working outside of an 
app context.


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 #4683: Fix bug with sorting columns in group by using time shift

2018-03-26 Thread GitBox
timifasubaa commented on issue #4683: Fix bug with sorting columns in group by 
using time shift
URL: 
https://github.com/apache/incubator-superset/pull/4683#issuecomment-376235345
 
 
   LGTM


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] hughhhh commented on a change in pull request #4680: Hotkeys in SQL Lab

2018-03-26 Thread GitBox
hug commented on a change in pull request #4680: Hotkeys in SQL Lab
URL: 
https://github.com/apache/incubator-superset/pull/4680#discussion_r177161126
 
 

 ##
 File path: superset/assets/javascripts/components/Hotkeys.jsx
 ##
 @@ -0,0 +1,53 @@
+import React from 'react';
+import PropTypes from 'prop-types';
+import { OverlayTrigger, Popover } from 'react-bootstrap';
+import { Table } from 'reactable';
+
+import Mousetrap from 'mousetrap';
+
+const propTypes = {
+  hotkeys: PropTypes.arrayOf(PropTypes.shape({
+key: PropTypes.string.isRequired,
+descr: PropTypes.string.isRequired,
+func: PropTypes.func.isRequired,
+  })).isRequired,
+  header: PropTypes.string,
+};
+
+const defaultProps = {
+  hotkeys: [],
+};
+
+export default class Hotkeys extends React.PureComponent {
+  componentDidMount() {
+this.props.hotkeys.forEach((keyConfig) => {
+  Mousetrap.bind([keyConfig.key], keyConfig.func);
+});
+  }
+  renderPopover() {
+return (
+  
+ ({
 
 Review comment:
   Nice


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] hughhhh commented on a change in pull request #4680: Hotkeys in SQL Lab

2018-03-26 Thread GitBox
hug commented on a change in pull request #4680: Hotkeys in SQL Lab
URL: 
https://github.com/apache/incubator-superset/pull/4680#discussion_r177160837
 
 

 ##
 File path: superset/assets/javascripts/SqlLab/actions.js
 ##
 @@ -196,7 +196,10 @@ export function setDatabases(databases) {
 }
 
 export function addQueryEditor(queryEditor) {
-  const newQe = Object.assign({}, queryEditor, { id: shortid.generate() });
+  const newQe = {
 
 Review comment:
   nit: `neQe >> newQuery`


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] hughhhh commented on a change in pull request #4680: Hotkeys in SQL Lab

2018-03-26 Thread GitBox
hug commented on a change in pull request #4680: Hotkeys in SQL Lab
URL: 
https://github.com/apache/incubator-superset/pull/4680#discussion_r177160837
 
 

 ##
 File path: superset/assets/javascripts/SqlLab/actions.js
 ##
 @@ -196,7 +196,10 @@ export function setDatabases(databases) {
 }
 
 export function addQueryEditor(queryEditor) {
-  const newQe = Object.assign({}, queryEditor, { id: shortid.generate() });
+  const newQe = {
 
 Review comment:
   nit: neQe >> newQuery


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] hughhhh commented on a change in pull request #4680: Hotkeys in SQL Lab

2018-03-26 Thread GitBox
hug commented on a change in pull request #4680: Hotkeys in SQL Lab
URL: 
https://github.com/apache/incubator-superset/pull/4680#discussion_r177160625
 
 

 ##
 File path: superset/assets/javascripts/SqlLab/components/SqlEditor.jsx
 ##
 @@ -226,6 +229,32 @@ class SqlEditor extends React.PureComponent {
   render() {
 const height = this.sqlEditorHeight();
 const defaultNorthHeight = this.props.queryEditor.height || 200;
+const SQLLAB_HOTKEYS = [
 
 Review comment:
   nit: Can we put this at the top of the file or even in config file for 
adding new hotkeys more easily. I'm like the idea of a having this separate and 
we import 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] mistercrunch closed pull request #4645: CRUD hints around SQL expressions

2018-03-26 Thread GitBox
mistercrunch closed pull request #4645: CRUD hints around SQL expressions
URL: https://github.com/apache/incubator-superset/pull/4645
 
 
   

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/connectors/sqla/views.py 
b/superset/connectors/sqla/views.py
index 8398cbcbdd..b667475033 100644
--- a/superset/connectors/sqla/views.py
+++ b/superset/connectors/sqla/views.py
@@ -57,8 +57,8 @@ class TableColumnInlineView(CompactCRUDMixin, 
SupersetModelView):  # noqa
 'expression-defined columns in some cases. In most case '
 'users should not need to alter this.'),
 'expression': utils.markdown(
-'a valid SQL expression as supported by the underlying backend. '
-'Example: `substr(name, 1, 1)`', True),
+'a valid, *non-aggregating* SQL expression as supported by the '
+'underlying backend. Example: `substr(name, 1, 1)`', True),
 'python_date_format': utils.markdown(Markup(
 'The pattern of timestamp format, use '
 'https://docs.python.org/2/library/'
@@ -114,8 +114,8 @@ class SqlMetricInlineView(CompactCRUDMixin, 
SupersetModelView):  # noqa
 'expression', 'table', 'd3format', 'is_restricted', 'warning_text']
 description_columns = {
 'expression': utils.markdown(
-'a valid SQL expression as supported by the underlying backend. '
-'Example: `count(DISTINCT userid)`', True),
+'a valid, *aggregating* SQL expression as supported by the '
+'underlying backend. Example: `count(DISTINCT userid)`', True),
 'is_restricted': _('Whether the access to this metric is restricted '
'to certain roles. Only roles with the permission '
"'metric access on XXX (the name of this metric)' "


 


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 #4680: Hotkeys in SQL Lab

2018-03-26 Thread GitBox
mistercrunch commented on issue #4680: Hotkeys in SQL Lab
URL: 
https://github.com/apache/incubator-superset/pull/4680#issuecomment-376232412
 
 
   removed the [WiP] tag, this is ready for review


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 #4288: Query starting with jinja2 statement is rejected because "it's not a select"

2018-03-26 Thread GitBox
mistercrunch closed issue #4288: Query starting with jinja2 statement is 
rejected because "it's not a select"
URL: https://github.com/apache/incubator-superset/issues/4288
 
 
   


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 #4686: Preprocess SQL Lab query prior to checking syntax

2018-03-26 Thread GitBox
mistercrunch closed pull request #4686: Preprocess SQL Lab query prior to 
checking syntax
URL: https://github.com/apache/incubator-superset/pull/4686
 
 
   

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/sql_lab.py b/superset/sql_lab.py
index 12644fcf80..d28de7c354 100644
--- a/superset/sql_lab.py
+++ b/superset/sql_lab.py
@@ -161,8 +161,18 @@ def handle_error(msg):
 if store_results and not results_backend:
 return handle_error("Results backend isn't configured.")
 
+try:
+template_processor = get_template_processor(
+database=database, query=query)
+tp = template_params or {}
+rendered_query = template_processor.process_template(query.sql, **tp)
+except Exception as e:
+logging.exception(e)
+msg = 'Template rendering failed: ' + utils.error_msg_from_exception(e)
+return handle_error(msg)
+
 # Limit enforced only for retrieving the data, not for the CTA queries.
-superset_query = SupersetQuery(query.sql)
+superset_query = SupersetQuery(rendered_query)
 executed_sql = superset_query.stripped()
 if not superset_query.is_select() and not database.allow_dml:
 return handle_error(
@@ -172,7 +182,6 @@ def handle_error(msg):
 return handle_error(
 'Only `SELECT` statements can be used with the CREATE TABLE '
 'feature.')
-return
 if not query.tmp_table_name:
 start_dttm = datetime.fromtimestamp(query.start_time)
 query.tmp_table_name = 'tmp_{}_table_{}'.format(
@@ -183,16 +192,6 @@ def handle_error(msg):
 db_engine_spec.limit_method == LimitMethod.WRAP_SQL):
 executed_sql = database.wrap_sql_limit(executed_sql, query.limit)
 query.limit_used = True
-try:
-template_processor = get_template_processor(
-database=database, query=query)
-tp = template_params or {}
-executed_sql = template_processor.process_template(
-executed_sql, **tp)
-except Exception as e:
-logging.exception(e)
-msg = 'Template rendering failed: ' + utils.error_msg_from_exception(e)
-return handle_error(msg)
 
 # Hook to allow environment-specific mutation (usually comments) to the SQL
 SQL_QUERY_MUTATOR = config.get('SQL_QUERY_MUTATOR')


 


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 #4684: Fix up the Lyft color scheme

2018-03-26 Thread GitBox
mistercrunch closed pull request #4684: Fix up the Lyft color scheme
URL: https://github.com/apache/incubator-superset/pull/4684
 
 
   

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/modules/colors.js 
b/superset/assets/javascripts/modules/colors.js
index 909a8bf0d8..1cd2b538bc 100644
--- a/superset/assets/javascripts/modules/colors.js
+++ b/superset/assets/javascripts/modules/colors.js
@@ -29,10 +29,16 @@ export const bnbColors = [
 ];
 
 export const lyftColors = [
-  '#ff00bf', // pink
-  '#352384', // purple
-  '#333447', // carbon
-  '#f3f3f5', // silver
+  '#EA0B8C',
+  '#6C838E',
+  '#29ABE2',
+  '#33D9C1',
+  '#9DACB9',
+  '#7560AA',
+  '#2D5584',
+  '#831C4A',
+  '#333D47',
+  '#AC2077',
 ];
 
 const d3Category10 = d3.scale.category10().range();
@@ -81,6 +87,7 @@ export const ALL_COLOR_SCHEMES = {
   d3Category20c,
   googleCategory10c,
   googleCategory20c,
+  lyftColors,
 };
 
 export const spectrums = {


 


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] jeffreythewang commented on issue #4299: Bump sqlalchemy to 1.2.2

2018-03-26 Thread GitBox
jeffreythewang commented on issue #4299: Bump sqlalchemy to 1.2.2
URL: 
https://github.com/apache/incubator-superset/pull/4299#issuecomment-376216211
 
 
   @xrmx Yup. I just opened one 
[here](https://github.com/LocusEnergy/sqlalchemy-vertica-python/issues/12), 
since it is dependent on which Vertica dialect you are using, and that is the 
one recommended in the docs.


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] jeffreythewang commented on issue #4299: Bump sqlalchemy to 1.2.2

2018-03-26 Thread GitBox
jeffreythewang commented on issue #4299: Bump sqlalchemy to 1.2.2
URL: 
https://github.com/apache/incubator-superset/pull/4299#issuecomment-376216211
 
 
   @xrmx Yeah I opened one 
[here](https://github.com/LocusEnergy/sqlalchemy-vertica-python/issues/12), 
since it is dependent on which Vertica dialect you are using, and that is the 
one recommended in the docs.


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] xrmx commented on issue #4692: dashboard empty

2018-03-26 Thread GitBox
xrmx commented on issue #4692: dashboard empty
URL: 
https://github.com/apache/incubator-superset/issues/4692#issuecomment-376212371
 
 
   This has already been fixed in a released version and there's already a 
duplicate issue #4685 open. Please close.


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] shyam2794 commented on issue #4667: Remove and Re-align the Bar values on clicking the legend

2018-03-26 Thread GitBox
shyam2794 commented on issue #4667: Remove and Re-align the Bar values on 
clicking the legend 
URL: 
https://github.com/apache/incubator-superset/issues/4667#issuecomment-37625
 
 
   Hi Team , 
   Any update on this .. 


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] lilila opened a new issue #4692: dashboard empty

2018-03-26 Thread GitBox
lilila opened a new issue #4692: dashboard empty
URL: https://github.com/apache/incubator-superset/issues/4692
 
 
   Make sure these boxes are checked before submitting your issue - thank you!
   
   - [x] I have checked the superset logs for python stacktraces and included 
it here as text if any
   - [x] I have reproduced the issue with at least the latest released version 
of superset
   - [x] I have checked the issue tracker for the same issue and I haven't 
found one similar
   
   
   ### Superset version
   23.3
   
   ### Expected results
   Dashboard with chart inside
   
   ### Actual results
   When I create a new dashboard it appears empty, to circumvent this problem I 
have to add manually [] in the "Position JSON" field.
   
   ### Steps to reproduce
   Create a new dashboard or create a new chart and add to a new dashboard 
   
   


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] lilila opened a new issue #4691: superset db upgrade fails

2018-03-26 Thread GitBox
lilila opened a new issue #4691: superset db upgrade fails
URL: https://github.com/apache/incubator-superset/issues/4691
 
 
   Make sure these boxes are checked before submitting your issue - thank you!
   
   - [x] I have checked the superset logs for python stacktraces and included 
it here as text if any
   - [x] I have reproduced the issue with at least the latest released version 
of superset
   - [x] I have checked the issue tracker for the same issue and I haven't 
found one similar
   
   
   
table clusters already exists 
   ### Superset version
   23.3
   
   ### Expected results
   
   Upgrade superset DB  with 
   
   > superset db upgrade
   
   after running 
   > pip install superset --upgrade
   ### Actual results
   
   > sqlalchemy.exc.OperationalError: (sqlite3.OperationalError) table clusters 
already exists [SQL: '\nCREATE TABLE clusters (\n\tcreated_on DATETIME NOT 
NULL, \n\tchanged_on DATETIME NOT NULL, \n\tid INTEGER NOT NULL, 
\n\tcluster_name VARCHAR(250), \n\tcoordinator_host VARCHAR(255), 
\n\tcoordinator_port INTEGER, \n\tcoordinator_endpoint VARCHAR(255), 
\n\tbroker_host VARCHAR(255), \n\tbroker_port INTEGER, \n\tbroker_endpoint 
VARCHAR(255), \n\tmetadata_last_refreshed DATETIME, \n\tcreated_by_fk INTEGER, 
\n\tchanged_by_fk INTEGER, \n\tPRIMARY KEY (id), \n\tUNIQUE (cluster_name), 
\n\tFOREIGN KEY(created_by_fk) REFERENCES ab_user (id), \n\tFOREIGN 
KEY(changed_by_fk) REFERENCES ab_user (id)\n)\n\n'] (Background on this error 
at: http://sqlalche.me/e/e3q8)
   
   ### Steps to reproduce
   
   > pip install superset --upgrade
   > superset db upgrade
   


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] b881659b commented on issue #4665: Inconsistent inclusion of a footer template

2018-03-26 Thread GitBox
b881659b commented on issue #4665: Inconsistent inclusion of a footer template
URL: 
https://github.com/apache/incubator-superset/issues/4665#issuecomment-376124639
 
 
   Thanks. For the benefit of anyone else looking for a similar solution: I 
replaced my existing patches with one adding the footer inclusion code to the 
`basic.html` file as suggested. It appears to be sufficient but I have not had 
a chance to test all sections. 
   
   I am happy for the issue to be closed if you do not intend to implement 
footer support in project-specific templates.


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] yamyamyuo commented on issue #1935: UnicodeEncodeError when show slice record

2018-03-26 Thread GitBox
yamyamyuo commented on issue #1935: UnicodeEncodeError when show slice record
URL: 
https://github.com/apache/incubator-superset/issues/1935#issuecomment-376106099
 
 
   > Using mysql? Tried setting the charset on the connection string as in 
mysql://user:password@1.1.1.1/db?charset=utf8? @mistercrunch 
   
   this can fix dashboard show record, however, slice show record is still 500 
error


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] Sidhartha03 opened a new issue #4690: css not loading at "http://0.0.0.0:8088/superset/welcome"

2018-03-26 Thread GitBox
Sidhartha03 opened a new issue #4690: css not loading at 
"http://0.0.0.0:8088/superset/welcome; 
URL: https://github.com/apache/incubator-superset/issues/4690
 
 
   Make sure these boxes are checked before submitting your issue - thank you!
   
   - [ ] I have checked the superset logs for python stacktraces and included 
it here as text if any
   - [ ] I have reproduced the issue with at least the latest released version 
of superset
   - [ ] I have checked the issue tracker for the same issue and I haven't 
found one similar
   
   
   ### Superset version
   
   
   ### Expected results
   
   
   ### Actual results
   
   
   ### Steps to reproduce
   
   
   


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