[GitHub] codecov-io commented on issue #4844: [docs] minor file name and format fix for the setup document

[GitBox]

codecov-io commented on issue #4844: [docs] minor file name and format fix for 
the setup document
URL: 
https://github.com/apache/incubator-superset/pull/4844#issuecomment-382605435
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4844?src=pr=h1)
 Report
   > Merging 
[#4844](https://codecov.io/gh/apache/incubator-superset/pull/4844?src=pr=desc)
 into 
[master](https://codecov.io/gh/apache/incubator-superset/commit/a98c3cfdefcde69d92c5dbe37c0343d3409c0f32?src=pr=desc)
 will **not change** coverage.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-superset/pull/4844/graphs/tree.svg?height=150=650=KsB0fHcx6l=pr)](https://codecov.io/gh/apache/incubator-superset/pull/4844?src=pr=tree)
   
   ```diff
   @@   Coverage Diff   @@
   ##   master#4844   +/-   ##
   ===
 Coverage   76.96%   76.96%   
   ===
 Files  44   44   
 Lines8534 8534   
   ===
 Hits 6568 6568   
 Misses   1966 1966
   ```
   
   
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4844?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/4844?src=pr=footer).
 Last update 
[a98c3cf...65f6720](https://codecov.io/gh/apache/incubator-superset/pull/4844?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 #4844: [docs] minor file name and format fix for the setup document

[GitBox]

codecov-io commented on issue #4844: [docs] minor file name and format fix for 
the setup document
URL: 
https://github.com/apache/incubator-superset/pull/4844#issuecomment-382605435
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4844?src=pr=h1)
 Report
   > Merging 
[#4844](https://codecov.io/gh/apache/incubator-superset/pull/4844?src=pr=desc)
 into 
[master](https://codecov.io/gh/apache/incubator-superset/commit/a98c3cfdefcde69d92c5dbe37c0343d3409c0f32?src=pr=desc)
 will **not change** coverage.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-superset/pull/4844/graphs/tree.svg?src=pr=KsB0fHcx6l=650=150)](https://codecov.io/gh/apache/incubator-superset/pull/4844?src=pr=tree)
   
   ```diff
   @@   Coverage Diff   @@
   ##   master#4844   +/-   ##
   ===
 Coverage   76.96%   76.96%   
   ===
 Files  44   44   
 Lines8534 8534   
   ===
 Hits 6568 6568   
 Misses   1966 1966
   ```
   
   
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4844?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/4844?src=pr=footer).
 Last update 
[a98c3cf...65f6720](https://codecov.io/gh/apache/incubator-superset/pull/4844?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 #4844: [docs] minor file name and format fix for the setup document

[GitBox]

codecov-io commented on issue #4844: [docs] minor file name and format fix for 
the setup document
URL: 
https://github.com/apache/incubator-superset/pull/4844#issuecomment-382605435
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4844?src=pr=h1)
 Report
   > Merging 
[#4844](https://codecov.io/gh/apache/incubator-superset/pull/4844?src=pr=desc)
 into 
[master](https://codecov.io/gh/apache/incubator-superset/commit/a98c3cfdefcde69d92c5dbe37c0343d3409c0f32?src=pr=desc)
 will **not change** coverage.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-superset/pull/4844/graphs/tree.svg?token=KsB0fHcx6l=650=pr=150)](https://codecov.io/gh/apache/incubator-superset/pull/4844?src=pr=tree)
   
   ```diff
   @@   Coverage Diff   @@
   ##   master#4844   +/-   ##
   ===
 Coverage   76.96%   76.96%   
   ===
 Files  44   44   
 Lines8534 8534   
   ===
 Hits 6568 6568   
 Misses   1966 1966
   ```
   
   
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4844?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/4844?src=pr=footer).
 Last update 
[a98c3cf...65f6720](https://codecov.io/gh/apache/incubator-superset/pull/4844?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] vnnw commented on issue #4734: Issue with Group by and columns in Pivot Table Viz

[GitBox]

vnnw commented on issue #4734: Issue with Group by and columns in Pivot Table 
Viz
URL: 
https://github.com/apache/incubator-superset/issues/4734#issuecomment-382604509
 
 
   Same problem 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] sekikn opened a new pull request #4844: [docs] minor fix for the setup document

[GitBox]

sekikn opened a new pull request #4844: [docs] minor fix for the setup document
URL: https://github.com/apache/incubator-superset/pull/4844
 
 
   


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] Neuliaus closed issue #4818: Viz Plugins and Widget Set

[GitBox]

Neuliaus closed issue #4818: Viz Plugins and Widget Set 
URL: https://github.com/apache/incubator-superset/issues/4818
 
 
   


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] qin4zhang commented on issue #4840: ldap configuration

[GitBox]

qin4zhang commented on issue #4840: ldap configuration
URL: 
https://github.com/apache/incubator-superset/issues/4840#issuecomment-382585378
 
 
   OK, thanks.


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] williaster opened a new pull request #4843: [formats] add better defaults for time + number formatting

[GitBox]

williaster opened a new pull request #4843: [formats] add better defaults for 
time + number formatting
URL: https://github.com/apache/incubator-superset/pull/4843
 
 
   This PR is all about making better default for time + number formatting   
It adds:
   
   - a more concise multi-format tick formatter. Here is the before and after 
(and you can play with it in [this observable 
notebook](https://beta.observablehq.com/d/cb85a1708c711939)):
 ![image 
1](https://user-images.githubusercontent.com/4496521/38965847-eb7f32fc-4333-11e8-94e9-e13794e8174c.png)
   
   - sets most default number formats to 1 decimal place instead of 3
   - reduces the number of y-axis ticks (to ~5), there is no need for ~10+ that 
nvd3 sets by default
   - rich tooltip => false by default, this makes tooltips look pretty bad, you 
can still hover over individual lines to see their values.
   
   @mistercrunch @hug @john-bodley @graceguo-supercat @michellethomas 
@fabianmenges 


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 #4842: Cleaning html tags from text

[GitBox]

codecov-io commented on issue #4842: Cleaning html tags from text
URL: 
https://github.com/apache/incubator-superset/pull/4842#issuecomment-382559635
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4842?src=pr=h1)
 Report
   > Merging 
[#4842](https://codecov.io/gh/apache/incubator-superset/pull/4842?src=pr=desc)
 into 
[master](https://codecov.io/gh/apache/incubator-superset/commit/a14dc26042ad533f34cefc605b9eb9b3597be1f8?src=pr=desc)
 will **not change** coverage.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-superset/pull/4842/graphs/tree.svg?src=pr=150=KsB0fHcx6l=650)](https://codecov.io/gh/apache/incubator-superset/pull/4842?src=pr=tree)
   
   ```diff
   @@   Coverage Diff   @@
   ##   master#4842   +/-   ##
   ===
 Coverage   76.93%   76.93%   
   ===
 Files  44   44   
 Lines8531 8531   
   ===
 Hits 6563 6563   
 Misses   1968 1968
   ```
   
   
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4842?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/4842?src=pr=footer).
 Last update 
[a14dc26...702bb41](https://codecov.io/gh/apache/incubator-superset/pull/4842?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 #4842: Cleaning html tags from text

[GitBox]

codecov-io commented on issue #4842: Cleaning html tags from text
URL: 
https://github.com/apache/incubator-superset/pull/4842#issuecomment-382559635
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4842?src=pr=h1)
 Report
   > Merging 
[#4842](https://codecov.io/gh/apache/incubator-superset/pull/4842?src=pr=desc)
 into 
[master](https://codecov.io/gh/apache/incubator-superset/commit/a14dc26042ad533f34cefc605b9eb9b3597be1f8?src=pr=desc)
 will **not change** coverage.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-superset/pull/4842/graphs/tree.svg?token=KsB0fHcx6l=150=650=pr)](https://codecov.io/gh/apache/incubator-superset/pull/4842?src=pr=tree)
   
   ```diff
   @@   Coverage Diff   @@
   ##   master#4842   +/-   ##
   ===
 Coverage   76.93%   76.93%   
   ===
 Files  44   44   
 Lines8531 8531   
   ===
 Hits 6563 6563   
 Misses   1968 1968
   ```
   
   
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4842?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/4842?src=pr=footer).
 Last update 
[a14dc26...702bb41](https://codecov.io/gh/apache/incubator-superset/pull/4842?src=pr=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] mistercrunch closed pull request #4821: Fix time granularity-related issues

[GitBox]

mistercrunch closed pull request #4821: Fix time granularity-related issues
URL: https://github.com/apache/incubator-superset/pull/4821
 
 
   

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/src/explore/stores/visTypes.js 
b/superset/assets/src/explore/stores/visTypes.js
index 473ee58454..4954cf0634 100644
--- a/superset/assets/src/explore/stores/visTypes.js
+++ b/superset/assets/src/explore/stores/visTypes.js
@@ -95,6 +95,15 @@ export const sections = {
   ],
 };
 
+const timeGrainSqlaAnimationOverrides = {
+  default: null,
+  mapStateToProps: state => ({
+choices: (state.datasource) ?
+  state.datasource.time_grain_sqla.filter(o => o[0] !== null) :
+  null,
+  }),
+};
+
 export const visTypes = {
   dist_bar: {
 label: t('Distribution - Bar Chart'),
@@ -555,6 +564,7 @@ export const visTypes = {
 description: t("Metric used as a weight for the grid's coloring"),
 validators: [v.nonEmpty],
   },
+  time_grain_sqla: timeGrainSqlaAnimationOverrides,
 },
   },
 
@@ -738,6 +748,7 @@ export const visTypes = {
   size: {
 validators: [],
   },
+  time_grain_sqla: timeGrainSqlaAnimationOverrides,
 },
   },
 
diff --git a/superset/connectors/sqla/models.py 
b/superset/connectors/sqla/models.py
index 7fbb7ae44d..9b2f682c2f 100644
--- a/superset/connectors/sqla/models.py
+++ b/superset/connectors/sqla/models.py
@@ -130,9 +130,10 @@ def get_timestamp_expression(self, time_grain):
 expr = db_spec.epoch_to_dttm().format(col=expr)
 elif pdf == 'epoch_ms':
 expr = db_spec.epoch_ms_to_dttm().format(col=expr)
-grain = self.table.database.grains_dict().get(time_grain, '{col}')
-expr = grain.function.format(col=expr)
-return literal_column(expr, type_=DateTime).label(DTTM_ALIAS)
+grain = self.table.database.grains_dict().get(time_grain)
+literal = grain.function if grain else '{col}'
+literal = expr.format(col=expr)
+return literal_column(literal, type_=DateTime).label(DTTM_ALIAS)
 
 @classmethod
 def import_obj(cls, i_column):
diff --git a/superset/data/__init__.py b/superset/data/__init__.py
index 9f505c12e9..160ed647f9 100644
--- a/superset/data/__init__.py
+++ b/superset/data/__init__.py
@@ -1301,7 +1301,7 @@ def load_deck_dash():
 "row_limit": 5000,
 "since": None,
 "size": "count",
-"time_grain_sqla": "Time Column",
+"time_grain_sqla": None,
 "until": None,
 "viewport": {
 "bearing": -4.952916738791771,
@@ -1359,7 +1359,7 @@ def load_deck_dash():
 },
 "point_radius_fixed": {"type": "fix", "value": 2000},
 "datasource": "5__table",
-"time_grain_sqla": "Time Column",
+"time_grain_sqla": None,
 "groupby": [],
 }
 print("Creating Screen Grid slice")
@@ -1408,7 +1408,7 @@ def load_deck_dash():
 "where": "",
 "point_radius_fixed": {"type": "fix", "value": 2000},
 "datasource": "5__table",
-"time_grain_sqla": "Time Column",
+"time_grain_sqla": None,
 "groupby": [],
 }
 print("Creating Hex slice")
@@ -1457,7 +1457,7 @@ def load_deck_dash():
 "where": "",
 "point_radius_fixed": {"type": "fix", "value": 2000},
 "datasource": "5__table",
-"time_grain_sqla": "Time Column",
+"time_grain_sqla": None,
 "groupby": [],
 }
 print("Creating Grid slice")
@@ -1474,62 +1474,62 @@ def load_deck_dash():
 polygon_tbl = db.session.query(TBL) \
 .filter_by(table_name='sf_population_polygons').first()
 slice_data = {
-"datasource": "11__table",
-"viz_type": "deck_polygon",
-"slice_id": 41,
-"granularity_sqla": None,
-"time_grain_sqla": None,
-"since": None,
-"until": None,
-"line_column": "contour",
-"line_type": "json",
-"mapbox_style": "mapbox://styles/mapbox/light-v9",
-"viewport": {
-"longitude": -122.43388541747726,
-"latitude": 37.752020331384834,
-"zoom": 11.133995608594631,
-"bearing": 37.89506450385642,
-"pitch": 60,
-"width": 667,
-"height": 906,
-"altitude": 1.5,
-"maxZoom": 20,
-"minZoom": 0,
-"maxPitch": 60,
-"minPitch": 0,
-"maxLatitude": 85.05113,
-"minLatitude": -85.05113
-},
-"reverse_long_lat": False,
-"fill_color_picker": {
-

[GitHub] williaster commented on a change in pull request #4839: Dashboard builder: fixed 1st round comments

[GitBox]

williaster commented on a change in pull request #4839: Dashboard builder: 
fixed 1st round comments
URL: 
https://github.com/apache/incubator-superset/pull/4839#discussion_r182594611
 
 

 ##
 File path: superset/assets/stylesheets/dashboard.less
 ##
 @@ -0,0 +1,260 @@
+@import "./less/cosmo/variables.less";
+
+.dashboard a i {
+  cursor: pointer;
+}
+.dashboard i.drag {
+  cursor: move !important;
+}
+.dashboard .slice-grid .preview-holder {
+  z-index: 1;
+  position: absolute;
+  background-color: #AAA;
+  border-color: #AAA;
+  opacity: 0.3;
+}
+.dashboard .widget {
+  position: absolute;
+  top: 16px;
+  left: 16px;
+  box-shadow: none;
+  background-color: transparent;
+  overflow: visible;
+}
+.dashboard .chart-header {
+  .dropdown.btn-group {
+position: absolute;
+top: 0;
+right: 0;
+  }
+
+  .dropdown-menu.dropdown-menu-right {
+right: 7px;
+top: -3px
+  }
+}
+
+.slice-header-controls-trigger {
+  border: 0;
+  padding: 0 0 0 20px;
+  background: none;
+  outline: none;
+  box-shadow: none;
+  color: #263238;
+
+  &.is-cached {
+color: red;
+  }
+
+  &:hover, &:focus {
+background: none;
+cursor: pointer;
+  }
+
+  .controls-container.dropdown-menu {
+top: 0;
+left: unset;
+right: 10px;
+
+&.is-open {
+  display: block;
+}
+
+& li {
+  white-space: nowrap;
+}
+  }
+}
+.slice-grid .slice_container {
+  background-color: #fff;
+}
+
+.dashboard .slice-grid .dragging,
+.dashboard .slice-grid .resizing {
+  opacity: 0.5;
+}
+.dashboard img.loading {
+  width: 20px;
+  margin: 5px;
+  position: absolute;
+}
+
+.dashboard .slice_title {
+  text-align: center;
+  font-weight: bold;
+  font-size: 14px;
+  padding: 5px;
+}
+.dashboard div.slice_content {
+  width: 100%;
+  height: 100%;
+}
+
+.modal img.loading {
+  width: 50px;
+  margin: 0;
+  position: relative;
+}
+
+.react-bs-container-body {
+  max-height: 400px;
+  overflow-y: auto;
+}
+
+.hidden, #pageDropDown {
+  display: none;
+}
+
+.slice-cell {
+  box-shadow: 0px 0px 20px 5px rgba(0,0,0,0);
+  transition: box-shadow 1s ease-in;
+
+  .dropdown,
+  .dropdown-menu {
+.fa {
+  font-size: 14px;
+}
+  }
+}
+
+.slice-cell-highlight {
+  box-shadow: 0px 0px 20px 5px rgba(0,0,0,0.2);
+  height: 100%;
+}
+
+.slice-cell .editable-title input[type="button"] {
+  font-weight: bold;
+}
+
+.chart-container {
+  box-sizing: border-box;
+}
+
+.chart-header .header {
+  font-size: 16px;
+  margin: 0 -10px;
+}
+.ace_gutter {
+z-index: 0;
+}
+.ace_content {
+z-index: 0;
+}
+.ace_scrollbar {
+z-index: 0;
+}
+.slice_container .alert {
+margin: 10px;
+}
+
+i.danger {
+  color: red;
+}
+
+i.warning {
+  color: orange;
+}
+
+.dashboard-builder-sidepane {
 
 Review comment:
   it'd be great if you could help make the css more modular and move all 
builder sidepane css to it's own `builder-sidepane.less` file. this is what I 
did for all of the new v2 components. large css files bloat over time.


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] williaster commented on a change in pull request #4839: Dashboard builder: fixed 1st round comments

[GitBox]

williaster commented on a change in pull request #4839: Dashboard builder: 
fixed 1st round comments
URL: 
https://github.com/apache/incubator-superset/pull/4839#discussion_r182594505
 
 

 ##
 File path: superset/assets/stylesheets/dashboard.less
 ##
 @@ -0,0 +1,260 @@
+@import "./less/cosmo/variables.less";
+
+.dashboard a i {
+  cursor: pointer;
+}
+.dashboard i.drag {
+  cursor: move !important;
+}
+.dashboard .slice-grid .preview-holder {
+  z-index: 1;
+  position: absolute;
+  background-color: #AAA;
+  border-color: #AAA;
+  opacity: 0.3;
+}
+.dashboard .widget {
+  position: absolute;
+  top: 16px;
+  left: 16px;
+  box-shadow: none;
+  background-color: transparent;
+  overflow: visible;
+}
+.dashboard .chart-header {
+  .dropdown.btn-group {
+position: absolute;
+top: 0;
+right: 0;
+  }
+
+  .dropdown-menu.dropdown-menu-right {
+right: 7px;
+top: -3px
+  }
+}
+
+.slice-header-controls-trigger {
+  border: 0;
+  padding: 0 0 0 20px;
+  background: none;
+  outline: none;
+  box-shadow: none;
+  color: #263238;
+
+  &.is-cached {
+color: red;
+  }
+
+  &:hover, &:focus {
+background: none;
+cursor: pointer;
+  }
+
+  .controls-container.dropdown-menu {
+top: 0;
+left: unset;
+right: 10px;
+
+&.is-open {
+  display: block;
+}
+
+& li {
+  white-space: nowrap;
+}
+  }
+}
+.slice-grid .slice_container {
+  background-color: #fff;
+}
+
+.dashboard .slice-grid .dragging,
+.dashboard .slice-grid .resizing {
+  opacity: 0.5;
+}
+.dashboard img.loading {
+  width: 20px;
+  margin: 5px;
+  position: absolute;
+}
+
+.dashboard .slice_title {
+  text-align: center;
+  font-weight: bold;
+  font-size: 14px;
+  padding: 5px;
+}
+.dashboard div.slice_content {
+  width: 100%;
+  height: 100%;
+}
+
+.modal img.loading {
+  width: 50px;
+  margin: 0;
+  position: relative;
+}
+
+.react-bs-container-body {
+  max-height: 400px;
+  overflow-y: auto;
+}
+
+.hidden, #pageDropDown {
+  display: none;
+}
+
+.slice-cell {
+  box-shadow: 0px 0px 20px 5px rgba(0,0,0,0);
+  transition: box-shadow 1s ease-in;
+
+  .dropdown,
+  .dropdown-menu {
+.fa {
+  font-size: 14px;
+}
+  }
+}
+
+.slice-cell-highlight {
+  box-shadow: 0px 0px 20px 5px rgba(0,0,0,0.2);
+  height: 100%;
+}
+
+.slice-cell .editable-title input[type="button"] {
+  font-weight: bold;
+}
+
+.chart-container {
+  box-sizing: border-box;
+}
+
+.chart-header .header {
+  font-size: 16px;
+  margin: 0 -10px;
+}
+.ace_gutter {
+z-index: 0;
+}
+.ace_content {
+z-index: 0;
+}
+.ace_scrollbar {
+z-index: 0;
+}
+.slice_container .alert {
+margin: 10px;
+}
+
+i.danger {
+  color: red;
+}
+
+i.warning {
+  color: orange;
+}
+
+.dashboard-builder-sidepane {
+  .trigger {
+height: 25px;
+width: 25px;
+color: #879399;
+position: relative;
+
+&.close {
+  top: 3px;
+}
+
+&.open {
+  position: absolute;
+  right: 14px;
+}
+  }
+}
+
+.component-layer {
+  .new-component.static {
+cursor: pointer;
 
 Review comment:
   if it's static should the pointer be `initial`?


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] williaster commented on a change in pull request #4839: Dashboard builder: fixed 1st round comments

[GitBox]

williaster commented on a change in pull request #4839: Dashboard builder: 
fixed 1st round comments
URL: 
https://github.com/apache/incubator-superset/pull/4839#discussion_r182594611
 
 

 ##
 File path: superset/assets/stylesheets/dashboard.less
 ##
 @@ -0,0 +1,260 @@
+@import "./less/cosmo/variables.less";
+
+.dashboard a i {
+  cursor: pointer;
+}
+.dashboard i.drag {
+  cursor: move !important;
+}
+.dashboard .slice-grid .preview-holder {
+  z-index: 1;
+  position: absolute;
+  background-color: #AAA;
+  border-color: #AAA;
+  opacity: 0.3;
+}
+.dashboard .widget {
+  position: absolute;
+  top: 16px;
+  left: 16px;
+  box-shadow: none;
+  background-color: transparent;
+  overflow: visible;
+}
+.dashboard .chart-header {
+  .dropdown.btn-group {
+position: absolute;
+top: 0;
+right: 0;
+  }
+
+  .dropdown-menu.dropdown-menu-right {
+right: 7px;
+top: -3px
+  }
+}
+
+.slice-header-controls-trigger {
+  border: 0;
+  padding: 0 0 0 20px;
+  background: none;
+  outline: none;
+  box-shadow: none;
+  color: #263238;
+
+  &.is-cached {
+color: red;
+  }
+
+  &:hover, &:focus {
+background: none;
+cursor: pointer;
+  }
+
+  .controls-container.dropdown-menu {
+top: 0;
+left: unset;
+right: 10px;
+
+&.is-open {
+  display: block;
+}
+
+& li {
+  white-space: nowrap;
+}
+  }
+}
+.slice-grid .slice_container {
+  background-color: #fff;
+}
+
+.dashboard .slice-grid .dragging,
+.dashboard .slice-grid .resizing {
+  opacity: 0.5;
+}
+.dashboard img.loading {
+  width: 20px;
+  margin: 5px;
+  position: absolute;
+}
+
+.dashboard .slice_title {
+  text-align: center;
+  font-weight: bold;
+  font-size: 14px;
+  padding: 5px;
+}
+.dashboard div.slice_content {
+  width: 100%;
+  height: 100%;
+}
+
+.modal img.loading {
+  width: 50px;
+  margin: 0;
+  position: relative;
+}
+
+.react-bs-container-body {
+  max-height: 400px;
+  overflow-y: auto;
+}
+
+.hidden, #pageDropDown {
+  display: none;
+}
+
+.slice-cell {
+  box-shadow: 0px 0px 20px 5px rgba(0,0,0,0);
+  transition: box-shadow 1s ease-in;
+
+  .dropdown,
+  .dropdown-menu {
+.fa {
+  font-size: 14px;
+}
+  }
+}
+
+.slice-cell-highlight {
+  box-shadow: 0px 0px 20px 5px rgba(0,0,0,0.2);
+  height: 100%;
+}
+
+.slice-cell .editable-title input[type="button"] {
+  font-weight: bold;
+}
+
+.chart-container {
+  box-sizing: border-box;
+}
+
+.chart-header .header {
+  font-size: 16px;
+  margin: 0 -10px;
+}
+.ace_gutter {
+z-index: 0;
+}
+.ace_content {
+z-index: 0;
+}
+.ace_scrollbar {
+z-index: 0;
+}
+.slice_container .alert {
+margin: 10px;
+}
+
+i.danger {
+  color: red;
+}
+
+i.warning {
+  color: orange;
+}
+
+.dashboard-builder-sidepane {
 
 Review comment:
   it'd be great if you could help make the css more modular and move all 
builder sidepane css to it's own `builder-sidepane.less` file


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] williaster commented on a change in pull request #4839: Dashboard builder: fixed 1st round comments

[GitBox]

williaster commented on a change in pull request #4839: Dashboard builder: 
fixed 1st round comments
URL: 
https://github.com/apache/incubator-superset/pull/4839#discussion_r182590483
 
 

 ##
 File path: 
superset/assets/javascripts/dashboard/util/dashboardLayoutConverter.js
 ##
 @@ -0,0 +1,331 @@
+import {
+  ROW_TYPE,
+  COLUMN_TYPE,
+  CHART_TYPE,
+  DASHBOARD_HEADER_TYPE,
+  DASHBOARD_ROOT_TYPE,
+  DASHBOARD_GRID_TYPE,
+} from '../v2/util/componentTypes';
+import {
+  DASHBOARD_GRID_ID,
+  DASHBOARD_HEADER_ID,
+  DASHBOARD_ROOT_ID,
+} from '../v2/util/constants';
+
+const MAX_RECURSIVE_LEVEL = 6;
+const GRID_RATIO = 4;
+const ROW_HEIGHT = 8;
+const generateId = function() {
+  let componentId = 1;
+  return () => (componentId++);
+}();
+
+/**
+ *
+ * @param positions: single array of slices
+ * @returns boundary object {top: number, bottom: number, left: number, right: 
number}
+ */
+function getBoundary(positions) {
+  let top = Number.MAX_VALUE, bottom = 0,
+left = Number.MAX_VALUE, right = 1;
+  positions.forEach(item => {
+const { row, col, size_x, size_y } = item;
+if (row <= top) top = row;
+if (col <= left ) left = col;
+if (bottom <= row + size_y) bottom = row + size_y;
+if (right <= col + size_x) right = col + size_x;
+  });
+
+  return {
+top,
+bottom,
+left,
+right
+  };
+}
+
+function getRowContainer() {
+  const id = 'DASHBOARD_ROW_TYPE-' + generateId();
+  return {
+version: 'v2',
+type: ROW_TYPE,
+id,
+children: [],
+meta: {
+  background: 'BACKGROUND_TRANSPARENT',
+},
+  };
+}
+
+function getColContainer() {
+  const id = 'DASHBOARD_COLUMN_TYPE-' + generateId();
+  return {
+version: 'v2',
+type: COLUMN_TYPE,
+id,
+children: [],
+meta: {
+  background: 'BACKGROUND_TRANSPARENT',
+},
+  };
+}
+
+function getChartHolder(item) {
+  const { row, col, size_x, size_y, slice_id } = item;
+  const converted = {
+row: Math.round(row / GRID_RATIO),
+col: Math.floor((col - 1) / GRID_RATIO) + 1,
+size_x: Math.max(1, Math.floor(size_x / GRID_RATIO)),
+size_y: Math.max(1, Math.round(size_y / GRID_RATIO)),
+slice_id,
+  };
+
+  return {
+version: 'v2',
+type: CHART_TYPE,
+id: 'DASHBOARD_CHART_TYPE-' + generateId(),
+children: [],
+meta: {
+  width: converted.size_x,
+  height: Math.round(converted.size_y * 100 / ROW_HEIGHT ),
+  chartId: slice_id,
+},
+  };
+}
+
+function getChildrenMax(items, attr, layout) {
+  return Math.max.apply(null, items.map(child => {
+return layout[child].meta[attr];
+  }));
+}
+
+function getChildrenSum(items, attr, layout) {
+  return items.reduce((preValue, child) => {
+return preValue + layout[child].meta[attr];
+  }, 0);
+}
+
+function sortByRowId(item1, item2) {
+  return item1.row - item2.row;
+}
+
+function sortByColId(item1, item2) {
+  return item1.col - item2.col;
+}
+
+function hasOverlap(positions, xAxis = true) {
+  return positions.slice()
+.sort(!!xAxis ? sortByColId : sortByRowId)
+.some((item, index, arr) => {
+  if (index === arr.length - 1) {
+return false;
+  }
+
+  if (!!xAxis) {
+return (item.col + item.size_x) > arr[index + 1].col;
+  } else {
+return (item.row + item.size_y) > arr[index + 1].row;
+  }
+});
+}
+
+function doConvert(positions, level, parent, root) {
+  if (positions.length === 0) {
+return;
+  }
+
+  if (positions.length === 1 || level >= MAX_RECURSIVE_LEVEL) {
+// special treatment for single chart dash, always wrap chart inside a row
+if (parent.type === 'DASHBOARD_GRID_TYPE') {
+  const rowContainer = getRowContainer();
+  root[rowContainer.id] = rowContainer;
+  parent.children.push(rowContainer.id);
+  parent = rowContainer;
+}
+
+const chartHolder = getChartHolder(positions[0]);
+root[chartHolder.id] = chartHolder;
+parent.children.push(chartHolder.id);
+return;
+  }
+
+  let currentItems = positions.slice();
+  const { top, bottom, left, right } = getBoundary(positions);
+  // find row dividers
+  const layers = [];
+  let currentRow = top + 1;
+  while (currentItems.length && currentRow <= bottom) {
+const upper = [],
+  lower = [];
+
+const isRowDivider = currentItems.every(item => {
+  const { row, col, size_x, size_y } = item;
+  if (row + size_y <= currentRow) {
+lower.push(item);
+return true;
+  } else if (row >= currentRow) {
+upper.push(item);
+return true;
+  } else {
+return false;
+  }
+});
+
+if (isRowDivider) {
+  currentItems = upper.slice();
+  layers.push(lower);
+}
+currentRow++;
+  }
+
+  layers.forEach((layer) => {
+if (layer.length === 0) {
+  return;
+}
+
+if (layer.length === 1) {
+  const chartHolder = getChartHolder(layer[0]);
+  root[chartHolder.id] = chartHolder;
+  parent.children.push(chartHolder.id);
+ 

[GitHub] williaster commented on a change in pull request #4839: Dashboard builder: fixed 1st round comments

[GitBox]

williaster commented on a change in pull request #4839: Dashboard builder: 
fixed 1st round comments
URL: 
https://github.com/apache/incubator-superset/pull/4839#discussion_r182590412
 
 

 ##
 File path: 
superset/assets/javascripts/dashboard/util/dashboardLayoutConverter.js
 ##
 @@ -0,0 +1,331 @@
+import {
+  ROW_TYPE,
+  COLUMN_TYPE,
+  CHART_TYPE,
+  DASHBOARD_HEADER_TYPE,
+  DASHBOARD_ROOT_TYPE,
+  DASHBOARD_GRID_TYPE,
+} from '../v2/util/componentTypes';
+import {
+  DASHBOARD_GRID_ID,
+  DASHBOARD_HEADER_ID,
+  DASHBOARD_ROOT_ID,
+} from '../v2/util/constants';
+
+const MAX_RECURSIVE_LEVEL = 6;
+const GRID_RATIO = 4;
+const ROW_HEIGHT = 8;
+const generateId = function() {
+  let componentId = 1;
+  return () => (componentId++);
+}();
+
+/**
+ *
+ * @param positions: single array of slices
+ * @returns boundary object {top: number, bottom: number, left: number, right: 
number}
+ */
+function getBoundary(positions) {
+  let top = Number.MAX_VALUE, bottom = 0,
+left = Number.MAX_VALUE, right = 1;
+  positions.forEach(item => {
+const { row, col, size_x, size_y } = item;
+if (row <= top) top = row;
+if (col <= left ) left = col;
+if (bottom <= row + size_y) bottom = row + size_y;
+if (right <= col + size_x) right = col + size_x;
+  });
+
+  return {
+top,
+bottom,
+left,
+right
+  };
+}
+
+function getRowContainer() {
+  const id = 'DASHBOARD_ROW_TYPE-' + generateId();
+  return {
+version: 'v2',
+type: ROW_TYPE,
+id,
+children: [],
+meta: {
+  background: 'BACKGROUND_TRANSPARENT',
+},
+  };
+}
+
+function getColContainer() {
+  const id = 'DASHBOARD_COLUMN_TYPE-' + generateId();
+  return {
+version: 'v2',
+type: COLUMN_TYPE,
+id,
+children: [],
+meta: {
+  background: 'BACKGROUND_TRANSPARENT',
+},
+  };
+}
+
+function getChartHolder(item) {
+  const { row, col, size_x, size_y, slice_id } = item;
+  const converted = {
+row: Math.round(row / GRID_RATIO),
+col: Math.floor((col - 1) / GRID_RATIO) + 1,
+size_x: Math.max(1, Math.floor(size_x / GRID_RATIO)),
+size_y: Math.max(1, Math.round(size_y / GRID_RATIO)),
+slice_id,
+  };
+
+  return {
+version: 'v2',
+type: CHART_TYPE,
+id: 'DASHBOARD_CHART_TYPE-' + generateId(),
+children: [],
+meta: {
+  width: converted.size_x,
+  height: Math.round(converted.size_y * 100 / ROW_HEIGHT ),
+  chartId: slice_id,
+},
+  };
+}
+
+function getChildrenMax(items, attr, layout) {
+  return Math.max.apply(null, items.map(child => {
+return layout[child].meta[attr];
+  }));
+}
+
+function getChildrenSum(items, attr, layout) {
+  return items.reduce((preValue, child) => {
+return preValue + layout[child].meta[attr];
+  }, 0);
+}
+
+function sortByRowId(item1, item2) {
+  return item1.row - item2.row;
+}
+
+function sortByColId(item1, item2) {
+  return item1.col - item2.col;
+}
+
+function hasOverlap(positions, xAxis = true) {
+  return positions.slice()
+.sort(!!xAxis ? sortByColId : sortByRowId)
+.some((item, index, arr) => {
+  if (index === arr.length - 1) {
+return false;
+  }
+
+  if (!!xAxis) {
+return (item.col + item.size_x) > arr[index + 1].col;
+  } else {
+return (item.row + item.size_y) > arr[index + 1].row;
+  }
+});
+}
+
+function doConvert(positions, level, parent, root) {
+  if (positions.length === 0) {
+return;
+  }
+
+  if (positions.length === 1 || level >= MAX_RECURSIVE_LEVEL) {
+// special treatment for single chart dash, always wrap chart inside a row
+if (parent.type === 'DASHBOARD_GRID_TYPE') {
+  const rowContainer = getRowContainer();
+  root[rowContainer.id] = rowContainer;
+  parent.children.push(rowContainer.id);
+  parent = rowContainer;
+}
+
+const chartHolder = getChartHolder(positions[0]);
+root[chartHolder.id] = chartHolder;
+parent.children.push(chartHolder.id);
+return;
+  }
+
+  let currentItems = positions.slice();
+  const { top, bottom, left, right } = getBoundary(positions);
+  // find row dividers
+  const layers = [];
+  let currentRow = top + 1;
+  while (currentItems.length && currentRow <= bottom) {
+const upper = [],
+  lower = [];
+
+const isRowDivider = currentItems.every(item => {
+  const { row, col, size_x, size_y } = item;
+  if (row + size_y <= currentRow) {
+lower.push(item);
+return true;
+  } else if (row >= currentRow) {
+upper.push(item);
+return true;
+  } else {
+return false;
+  }
+});
+
+if (isRowDivider) {
+  currentItems = upper.slice();
+  layers.push(lower);
+}
+currentRow++;
+  }
+
+  layers.forEach((layer) => {
+if (layer.length === 0) {
+  return;
+}
+
+if (layer.length === 1) {
+  const chartHolder = getChartHolder(layer[0]);
+  root[chartHolder.id] = chartHolder;
+  parent.children.push(chartHolder.id);
+ 

[GitHub] williaster commented on a change in pull request #4839: Dashboard builder: fixed 1st round comments

[GitBox]

williaster commented on a change in pull request #4839: Dashboard builder: 
fixed 1st round comments
URL: 
https://github.com/apache/incubator-superset/pull/4839#discussion_r182590121
 
 

 ##
 File path: superset/assets/javascripts/dashboard/reducers/sliceEntities.js
 ##
 @@ -0,0 +1,57 @@
+import * as actions from '../actions/sliceEntities';
 
 Review comment:
   nit -- just import actual functions


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] michellethomas opened a new pull request #4842: Cleaning html tags from text

[GitBox]

michellethomas opened a new pull request #4842: Cleaning html tags from text
URL: https://github.com/apache/incubator-superset/pull/4842
 
 
   There are a few cases where we are using d3.html() which intentionally 
doesn't escape html. In these cases if a user has data with html tags we are 
not escaping it so some js can be executed. For example if a group by column in 
a table has an html tag with an onerror, the onerror will get executed in the 
browser when the table renders.
   
   Initially I tried to do this in some central place so that we didn't have to 
go into individual files in /visualizations but when looking into it more, it 
seems to happen when we are using d3.html(). Let me know if there's a better 
way to do this. I may have missed a few cases, mainly trying to quickly get a 
fix out for the most used visualizations.
   
   @mistercrunch @graceguo-supercat 


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] williaster commented on a change in pull request #4839: Dashboard builder: fixed 1st round comments

[GitBox]

williaster commented on a change in pull request #4839: Dashboard builder: 
fixed 1st round comments
URL: 
https://github.com/apache/incubator-superset/pull/4839#discussion_r182589899
 
 

 ##
 File path: superset/assets/javascripts/dashboard/reducers/initState.js
 ##
 @@ -0,0 +1,106 @@
+import { chart } from '../../chart/chartReducer';
+import { initSliceEntities } from './sliceEntities';
+import { getParam } from '../../modules/utils';
+import { applyDefaultFormData } from '../../explore/stores/store';
+import { getColorFromScheme } from '../../modules/colors';
+import layoutConverter from '../util/dashboardLayoutConverter';
+
+export default function(bootstrapData) {
+  const { user_id, datasources, common } = bootstrapData;
+  delete common.locale;
+  delete common.language_pack;
+
+  const dashboard = { ...bootstrapData.dashboard_data };
+  let filters = {};
+  try {
+// allow request parameter overwrite dashboard metadata
+filters = JSON.parse(getParam('preselect_filters') || 
dashboard.metadata.default_filters);
+  } catch (e) {
+//
+  }
+
+  // Priming the color palette with user's label-color mapping provided in
+  // the dashboard's JSON metadata
+  if (dashboard.metadata && dashboard.metadata.label_colors) {
+const colorMap = dashboard.metadata.label_colors;
+for (const label in colorMap) {
+  getColorFromScheme(label, null, colorMap[label]);
+}
+  }
+
+  // dashboard layout
+  const positionJson = dashboard.position_json;
+  let layout;
+  if (!positionJson || !positionJson['DASHBOARD_ROOT_ID']) {
+layout = layoutConverter(dashboard);
+  } else {
+layout = positionJson;
+  }
+
+  const dashboardLayout = {
+past: [],
+present: layout,
+future: [],
+  };
+  delete dashboard.position_json;
+  delete dashboard.css;
+
+  // charts: dynamic queries
 
 Review comment:
   can you just rename these to `chartQueries` and `sliceEntities` so you 
wouldn't need a comment? you could then just set the return object to `chart: 
chartQueries`


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] williaster commented on a change in pull request #4839: Dashboard builder: fixed 1st round comments

[GitBox]

williaster commented on a change in pull request #4839: Dashboard builder: 
fixed 1st round comments
URL: 
https://github.com/apache/incubator-superset/pull/4839#discussion_r182590044
 
 

 ##
 File path: superset/assets/javascripts/dashboard/reducers/initState.js
 ##
 @@ -0,0 +1,106 @@
+import { chart } from '../../chart/chartReducer';
+import { initSliceEntities } from './sliceEntities';
+import { getParam } from '../../modules/utils';
+import { applyDefaultFormData } from '../../explore/stores/store';
+import { getColorFromScheme } from '../../modules/colors';
+import layoutConverter from '../util/dashboardLayoutConverter';
+
+export default function(bootstrapData) {
+  const { user_id, datasources, common } = bootstrapData;
+  delete common.locale;
+  delete common.language_pack;
+
+  const dashboard = { ...bootstrapData.dashboard_data };
+  let filters = {};
+  try {
+// allow request parameter overwrite dashboard metadata
+filters = JSON.parse(getParam('preselect_filters') || 
dashboard.metadata.default_filters);
+  } catch (e) {
+//
+  }
+
+  // Priming the color palette with user's label-color mapping provided in
+  // the dashboard's JSON metadata
+  if (dashboard.metadata && dashboard.metadata.label_colors) {
+const colorMap = dashboard.metadata.label_colors;
+for (const label in colorMap) {
+  getColorFromScheme(label, null, colorMap[label]);
+}
+  }
+
+  // dashboard layout
+  const positionJson = dashboard.position_json;
+  let layout;
+  if (!positionJson || !positionJson['DASHBOARD_ROOT_ID']) {
+layout = layoutConverter(dashboard);
+  } else {
+layout = positionJson;
+  }
+
+  const dashboardLayout = {
+past: [],
+present: layout,
+future: [],
+  };
+  delete dashboard.position_json;
+  delete dashboard.css;
+
+  // charts: dynamic queries
+  // slices: saved data from slices table
+  const charts = {},
+slices = {},
+sliceIds = new Set();
+  dashboard.slices.forEach((slice) => {
+const key = slice.slice_id;
+charts[key] = { ...chart,
+  id: key,
+  form_data: slice.form_data,
+  formData: applyDefaultFormData(slice.form_data),
+};
+
+slices[key] = {
+  slice_id: key,
+  slice_url: slice.slice_url,
+  slice_name: slice.slice_name,
+  form_data: slice.form_data,
+  edit_url: slice.edit_url,
+  viz_type: slice.form_data.viz_type,
+  datasource: slice.form_data.datasource,
+  description: slice.description,
+  description_markeddown: slice.description_markeddown,
+};
+
+sliceIds.add(key);
+  });
+
+  return {
 
 Review comment:
   <3 how readable this is 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] williaster commented on a change in pull request #4839: Dashboard builder: fixed 1st round comments

[GitBox]

williaster commented on a change in pull request #4839: Dashboard builder: 
fixed 1st round comments
URL: 
https://github.com/apache/incubator-superset/pull/4839#discussion_r182589479
 
 

 ##
 File path: superset/assets/javascripts/dashboard/reducers/initState.js
 ##
 @@ -0,0 +1,106 @@
+import { chart } from '../../chart/chartReducer';
+import { initSliceEntities } from './sliceEntities';
+import { getParam } from '../../modules/utils';
+import { applyDefaultFormData } from '../../explore/stores/store';
+import { getColorFromScheme } from '../../modules/colors';
+import layoutConverter from '../util/dashboardLayoutConverter';
+
+export default function(bootstrapData) {
 
 Review comment:
   I think you'll get some .eslint errors for this soon, but debugging is 
easier if you name your functions


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] williaster commented on a change in pull request #4839: Dashboard builder: fixed 1st round comments

[GitBox]

williaster commented on a change in pull request #4839: Dashboard builder: 
fixed 1st round comments
URL: 
https://github.com/apache/incubator-superset/pull/4839#discussion_r182589375
 
 

 ##
 File path: superset/assets/javascripts/dashboard/reducers/index.js
 ##
 @@ -0,0 +1,29 @@
+import { combineReducers } from 'redux';
+import shortid from 'shortid';
+
+import charts from '../../chart/chartReducer';
+import dashboardState from './dashboard';
 
 Review comment:
   I'd update the file to be called `dashboardState` too


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] williaster commented on a change in pull request #4839: Dashboard builder: fixed 1st round comments

[GitBox]

williaster commented on a change in pull request #4839: Dashboard builder: 
fixed 1st round comments
URL: 
https://github.com/apache/incubator-superset/pull/4839#discussion_r182589335
 
 

 ##
 File path: superset/assets/javascripts/dashboard/reducers/index.js
 ##
 @@ -0,0 +1,29 @@
+import { combineReducers } from 'redux';
+import shortid from 'shortid';
+
+import charts from '../../chart/chartReducer';
+import dashboardState from './dashboard';
+import datasources from './datasources';
+import sliceEntities from './sliceEntities';
+import dashboardLayout from '../v2/reducers/index';
 
 Review comment:
   should you update the `/v2/reducers/index` file to be called 
`dashboardLayout`?


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] williaster commented on a change in pull request #4839: Dashboard builder: fixed 1st round comments

[GitBox]

williaster commented on a change in pull request #4839: Dashboard builder: 
fixed 1st round comments
URL: 
https://github.com/apache/incubator-superset/pull/4839#discussion_r182589180
 
 

 ##
 File path: superset/assets/javascripts/dashboard/reducers/dashboard.js
 ##
 @@ -0,0 +1,115 @@
+import { merge as mergeArray } from 'd3';
+
+import * as actions from '../actions/dashboard';
 
 Review comment:
   I would import all of these individually. also, should this file (similar to 
actions) be called `dashboardState`?


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] williaster commented on a change in pull request #4839: Dashboard builder: fixed 1st round comments

[GitBox]

williaster commented on a change in pull request #4839: Dashboard builder: 
fixed 1st round comments
URL: 
https://github.com/apache/incubator-superset/pull/4839#discussion_r182588953
 
 

 ##
 File path: superset/assets/javascripts/dashboard/index.jsx
 ##
 @@ -8,37 +8,19 @@ import { initEnhancer } from '../reduxUtils';
 import { appSetup } from '../common';
 import { initJQueryAjax } from '../modules/utils';
 import DashboardContainer from './components/DashboardContainer';
-// import rootReducer, { getInitialState } from './reducers';
-
-import emptyDashboardLayout from './v2/fixtures/emptyDashboardLayout';
-import rootReducer from './v2/reducers/';
+import getInitialState from './reducers/initState';
 
 Review comment:
   but love having it in its own file!  


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] williaster commented on a change in pull request #4839: Dashboard builder: fixed 1st round comments

[GitBox]

williaster commented on a change in pull request #4839: Dashboard builder: 
fixed 1st round comments
URL: 
https://github.com/apache/incubator-superset/pull/4839#discussion_r18251
 
 

 ##
 File path: superset/assets/javascripts/dashboard/index.jsx
 ##
 @@ -8,37 +8,19 @@ import { initEnhancer } from '../reduxUtils';
 import { appSetup } from '../common';
 import { initJQueryAjax } from '../modules/utils';
 import DashboardContainer from './components/DashboardContainer';
-// import rootReducer, { getInitialState } from './reducers';
-
-import emptyDashboardLayout from './v2/fixtures/emptyDashboardLayout';
-import rootReducer from './v2/reducers/';
+import getInitialState from './reducers/initState';
 
 Review comment:
   nit -- I would call this `getInitialState`, it's not clear it's a function 
(vs an object) and there's no benefit in saving 3 characters by calling it 
`init` instead of `initial`


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] williaster commented on a change in pull request #4839: Dashboard builder: fixed 1st round comments

[GitBox]

williaster commented on a change in pull request #4839: Dashboard builder: 
fixed 1st round comments
URL: 
https://github.com/apache/incubator-superset/pull/4839#discussion_r182587996
 
 

 ##
 File path: superset/assets/javascripts/dashboard/components/SliceAdder.jsx
 ##
 @@ -1,219 +1,215 @@
 import React from 'react';
-import $ from 'jquery';
 import PropTypes from 'prop-types';
-import { BootstrapTable, TableHeaderColumn } from 'react-bootstrap-table';
+import cx from 'classnames';
+import { DropdownButton, MenuItem } from 'react-bootstrap';
+import { List } from 'react-virtualized';
+import SearchInput, {createFilter} from 'react-search-input';
 
-import ModalTrigger from '../../components/ModalTrigger';
-import { t } from '../../locales';
-
-require('react-bootstrap-table/css/react-bootstrap-table.css');
+import DragDroppable from '../v2/components/dnd/DragDroppable';
+import { CHART_TYPE, NEW_COMPONENT_SOURCE_TYPE } from 
'../v2/util/componentTypes';
+import { NEW_CHART_ID, NEW_COMPONENTS_SOURCE_ID } from '../v2/util/constants';
+import { slicePropShape } from '../v2/util/propShapes';
 
 const propTypes = {
-  dashboard: PropTypes.object.isRequired,
-  triggerNode: PropTypes.node.isRequired,
+  actions: PropTypes.object,
 
 Review comment:
   it'd be great to move away from this pattern of not defining explicit action 
shapes, can be error prone and harder to refactor than it should be. not a 
blocker for this but fyi.


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] williaster commented on a change in pull request #4839: Dashboard builder: fixed 1st round comments

[GitBox]

williaster commented on a change in pull request #4839: Dashboard builder: 
fixed 1st round comments
URL: 
https://github.com/apache/incubator-superset/pull/4839#discussion_r182587996
 
 

 ##
 File path: superset/assets/javascripts/dashboard/components/SliceAdder.jsx
 ##
 @@ -1,219 +1,215 @@
 import React from 'react';
-import $ from 'jquery';
 import PropTypes from 'prop-types';
-import { BootstrapTable, TableHeaderColumn } from 'react-bootstrap-table';
+import cx from 'classnames';
+import { DropdownButton, MenuItem } from 'react-bootstrap';
+import { List } from 'react-virtualized';
+import SearchInput, {createFilter} from 'react-search-input';
 
-import ModalTrigger from '../../components/ModalTrigger';
-import { t } from '../../locales';
-
-require('react-bootstrap-table/css/react-bootstrap-table.css');
+import DragDroppable from '../v2/components/dnd/DragDroppable';
+import { CHART_TYPE, NEW_COMPONENT_SOURCE_TYPE } from 
'../v2/util/componentTypes';
+import { NEW_CHART_ID, NEW_COMPONENTS_SOURCE_ID } from '../v2/util/constants';
+import { slicePropShape } from '../v2/util/propShapes';
 
 const propTypes = {
-  dashboard: PropTypes.object.isRequired,
-  triggerNode: PropTypes.node.isRequired,
+  actions: PropTypes.object,
 
 Review comment:
   it'd be great to move away from this pattern of not defining explicit action 
shapes, can be error prone and extremely hard to refactor. not a blocker for 
this but fyi.


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] williaster commented on a change in pull request #4839: Dashboard builder: fixed 1st round comments

[GitBox]

williaster commented on a change in pull request #4839: Dashboard builder: 
fixed 1st round comments
URL: 
https://github.com/apache/incubator-superset/pull/4839#discussion_r182587270
 
 

 ##
 File path: superset/assets/javascripts/dashboard/components/Dashboard.jsx
 ##
 @@ -3,81 +3,69 @@ import PropTypes from 'prop-types';
 
 import AlertsWrapper from '../../components/AlertsWrapper';
 import GridLayout from './GridLayout';
-import Header from './Header';
+import {
+  chartPropShape,
+  slicePropShape,
+  dashboardInfoPropShape,
+  dashboardStatePropShape,
+} from '../v2/util/propShapes';
 
 Review comment:
    


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] williaster commented on a change in pull request #4839: Dashboard builder: fixed 1st round comments

[GitBox]

williaster commented on a change in pull request #4839: Dashboard builder: 
fixed 1st round comments
URL: 
https://github.com/apache/incubator-superset/pull/4839#discussion_r182586938
 
 

 ##
 File path: superset/assets/javascripts/dashboard/actions/sliceEntities.js
 ##
 @@ -0,0 +1,91 @@
+/* global notify */
+import $ from 'jquery';
+
+export const UPDATE_SLICE_NAME = 'UPDATE_SLICE_NAME';
+export function updateSliceName(key, sliceName) {
+  return { type: UPDATE_SLICE_NAME, key, sliceName };
+}
+
+export function saveSliceName(slice, sliceName) {
+  const oldName = slice.slice_name;
+  return (dispatch) => {
+const sliceParams = {};
+sliceParams.slice_id = slice.slice_id;
+sliceParams.action = 'overwrite';
+sliceParams.slice_name = sliceName;
+
+const url = slice.slice_url + '&' +
+  Object.keys(sliceParams)
+  .map(key => (key + '=' + sliceParams[key]))
+  .join('&');
+const key = slice.slice_id;
+return $.ajax({
+  url,
+  type: 'POST',
+  success: () => {
+dispatch(updateSliceName(key, sliceName));
+notify.success('This slice name was saved successfully.');
+  },
+  error: () => {
+// if server-side reject the overwrite action,
+// revert to old state
+dispatch(updateSliceName(key, oldName));
+notify.error("You don't have the rights to alter this slice");
+  },
+});
+  };
+}
+
+export const SET_ALL_SLICES = 'SET_ALL_SLICES';
+export function setAllSlices(slices) {
+  return { type: SET_ALL_SLICES, slices };
+}
+
+export const FETCH_ALL_SLICES_STARTED = 'FETCH_ALL_SLICES_STARTED';
+export function fetchAllSlicesStarted() {
+  return { type: FETCH_ALL_SLICES_STARTED };
+}
+
+export const FETCH_ALL_SLICES_FAILED = 'FETCH_ALL_SLICES_FAILED';
+export function fetchAllSlicesFailed(error) {
+  return { type: FETCH_ALL_SLICES_FAILED, error };
+}
+
+export function fetchAllSlices(userId) {
+  return (dispatch, getState) => {
+const { sliceEntities }  = getState();
+if (sliceEntities.lastUpdated === 0) {
+  dispatch(fetchAllSlicesStarted());
+
+  const uri = `/sliceaddview/api/read?_flt_0_created_by=${userId}`;
+  return $.ajax({
+url: uri,
+type: 'GET',
+success: response => {
+  const slices = {};
+  response.result.forEach(slice => {
+const form_data = JSON.parse(slice.params);
+slices[slice.id] = {
+  slice_id: slice.id,
+  slice_url: slice.slice_url,
+  slice_name: slice.slice_name,
+  edit_url: slice.edit_url,
+  form_data,
+  datasource: form_data.datasource,
 
 Review comment:
   why do we copy this if the slice already has `form_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] williaster commented on a change in pull request #4839: Dashboard builder: fixed 1st round comments

[GitBox]

williaster commented on a change in pull request #4839: Dashboard builder: 
fixed 1st round comments
URL: 
https://github.com/apache/incubator-superset/pull/4839#discussion_r182586821
 
 

 ##
 File path: superset/assets/javascripts/dashboard/actions/sliceEntities.js
 ##
 @@ -0,0 +1,91 @@
+/* global notify */
 
 Review comment:
   should we update this to use the new toast actions? can be in a future PR if 
you want.


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 #4833: [sqllab] Help sqllab forget query history

[GitBox]

john-bodley commented on issue #4833: [sqllab] Help sqllab forget query history
URL: 
https://github.com/apache/incubator-superset/pull/4833#issuecomment-382543787
 
 
   @timifasubaa I wonder if `timeout` or maybe `timedout`(past tense) is more 
descriptive that `failed` since the later can mean multiple things.


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 #4833: [sqllab] Help sqllab forget query history

[GitBox]

john-bodley commented on issue #4833: [sqllab] Help sqllab forget query history
URL: 
https://github.com/apache/incubator-superset/pull/4833#issuecomment-382543787
 
 
   @timifasubaa I wonder if `timedout` is more descriptive that `failed` since 
the later can mean multiple things.


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 #4829: [import csv] ensure directory exists before saving csv file

[GitBox]

john-bodley closed pull request #4829: [import csv] ensure directory exists 
before saving csv file
URL: https://github.com/apache/incubator-superset/pull/4829
 
 
   

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/utils.py b/superset/utils.py
index d465bb086c..2d06c48f22 100644
--- a/superset/utils.py
+++ b/superset/utils.py
@@ -12,6 +12,7 @@
 from email.mime.multipart import MIMEMultipart
 from email.mime.text import MIMEText
 from email.utils import formatdate
+import errno
 import functools
 import json
 import logging
@@ -827,3 +828,11 @@ def is_adhoc_metric(metric):
 
 def get_metric_names(metrics):
 return [metric['label'] if is_adhoc_metric(metric) else metric for metric 
in metrics]
+
+
+def ensure_path_exists(path):
+try:
+os.makedirs(path)
+except OSError as exc:
+if not (os.path.isdir(path) and exc.errno == errno.EEXIST):
+raise
diff --git a/superset/views/core.py b/superset/views/core.py
index 19885dcee3..f66b70ef55 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -347,15 +347,17 @@ def form_post(self, form):
 csv_file = form.csv_file.data
 form.csv_file.data.filename = 
secure_filename(form.csv_file.data.filename)
 csv_filename = form.csv_file.data.filename
+path = os.path.join(config['UPLOAD_FOLDER'], csv_filename)
 try:
-csv_file.save(os.path.join(config['UPLOAD_FOLDER'], csv_filename))
+utils.ensure_path_exists(config['UPLOAD_FOLDER'])
+csv_file.save(path)
 table = SqlaTable(table_name=form.name.data)
 table.database = form.data.get('con')
 table.database_id = table.database.id
 table.database.db_engine_spec.create_table_from_csv(form, table)
 except Exception as e:
 try:
-os.remove(os.path.join(config['UPLOAD_FOLDER'], csv_filename))
+os.remove(path)
 except OSError:
 pass
 message = 'Table name {} already exists. Please pick 
another'.format(
@@ -365,7 +367,7 @@ def form_post(self, form):
 'danger')
 return redirect('/csvtodatabaseview/form')
 
-os.remove(os.path.join(config['UPLOAD_FOLDER'], csv_filename))
+os.remove(path)
 # Go back to welcome page / splash screen
 db_name = table.database.database_name
 message = _('CSV file "{0}" uploaded to table "{1}" in '


 


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 issue #4576: Upload a csv via gui points to non existing folder: '.../superset/app/static/uploads/data.csv'

[GitBox]

john-bodley closed issue #4576: Upload a csv via gui points to non existing 
folder: '.../superset/app/static/uploads/data.csv'
URL: https://github.com/apache/incubator-superset/issues/4576
 
 
   


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] betodealmeida commented on a change in pull request #4821: Fix time granularity-related issues

[GitBox]

betodealmeida commented on a change in pull request #4821: Fix time 
granularity-related issues
URL: 
https://github.com/apache/incubator-superset/pull/4821#discussion_r182581875
 
 

 ##
 File path: superset/models/core.py
 ##
 @@ -786,7 +786,12 @@ def grains(self):
 return self.db_engine_spec.time_grains
 
 def grains_dict(self):
-return {grain.duration: grain for grain in self.grains()}
+"""Allowing to lookup grain by either label or duration
+
+For backward compatibility"""
+d = {grain.duration: grain for grain in self.grains()}
+d.update({grain.label: grain for grain in self.grains()})
 
 Review comment:
   Awesome, thanks!


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] williaster commented on a change in pull request #4839: Dashboard builder: fixed 1st round comments

[GitBox]

williaster commented on a change in pull request #4839: Dashboard builder: 
fixed 1st round comments
URL: 
https://github.com/apache/incubator-superset/pull/4839#discussion_r182575745
 
 

 ##
 File path: superset/assets/javascripts/dashboard/actions/dashboard.js
 ##
 @@ -0,0 +1,165 @@
+import $ from 'jquery';
 
 Review comment:
   should this file be called `dashboardState`?


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 #4833: [sqllab] Help sqllab forget query history

[GitBox]

timifasubaa commented on issue #4833: [sqllab] Help sqllab forget query history
URL: 
https://github.com/apache/incubator-superset/pull/4833#issuecomment-382532775
 
 
   Okay, I will change the logic to set all pending or running queries older 
than 6 hours old to failed. 


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 #4833: [sqllab] Help sqllab forget query history

[GitBox]

codecov-io commented on issue #4833: [sqllab] Help sqllab forget query history
URL: 
https://github.com/apache/incubator-superset/pull/4833#issuecomment-382534692
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4833?src=pr=h1)
 Report
   > Merging 
[#4833](https://codecov.io/gh/apache/incubator-superset/pull/4833?src=pr=desc)
 into 
[master](https://codecov.io/gh/apache/incubator-superset/commit/eac97ce9f201be1c8245aa9ad452c05f702f8fd3?src=pr=desc)
 will **not change** coverage.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-superset/pull/4833/graphs/tree.svg?token=KsB0fHcx6l=pr=650=150)](https://codecov.io/gh/apache/incubator-superset/pull/4833?src=pr=tree)
   
   ```diff
   @@   Coverage Diff   @@
   ##   master#4833   +/-   ##
   ===
 Coverage   76.91%   76.91%   
   ===
 Files  44   44   
 Lines8522 8522   
   ===
 Hits 6555 6555   
 Misses   1967 1967
   ```
   
   
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4833?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/4833?src=pr=footer).
 Last update 
[eac97ce...e47d8a5](https://codecov.io/gh/apache/incubator-superset/pull/4833?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 issue #4833: [sqllab] Help sqllab forget query history

[GitBox]

timifasubaa commented on issue #4833: [sqllab] Help sqllab forget query history
URL: 
https://github.com/apache/incubator-superset/pull/4833#issuecomment-382532775
 
 
   Okay, I will change the logic to set all pengind or running queries more 
than 6 hours old to failed. 


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] Tresdon commented on issue #4725: [WIP] Add "Published" feature to dashboards

[GitBox]

Tresdon commented on issue #4725: [WIP] Add "Published" feature to dashboards
URL: 
https://github.com/apache/incubator-superset/pull/4725#issuecomment-382530870
 
 
   Hey @daniel5001 ,
   
   This feature is still in the works and hasn't been forgotten, I'm currently 
working on ironing out a few bugs I've found as a result of adding an icon to 
toggle the status in the UI (and just trying to make it pleasant since it would 
show up on every dashboard) but other than that it's becoming pretty close to 
being finished!


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 #4829: [import csv] ensure directory exists before saving csv file

[GitBox]

timifasubaa commented on issue #4829: [import csv] ensure directory exists 
before saving csv file
URL: 
https://github.com/apache/incubator-superset/pull/4829#issuecomment-382529887
 
 
   @john-bodley Succeeded. Ready for merge


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 #4834: [WIP] Load async sql lab results early for Presto

[GitBox]

timifasubaa commented on issue #4834: [WIP] Load async sql lab results early 
for Presto
URL: 
https://github.com/apache/incubator-superset/pull/4834#issuecomment-382529350
 
 
   @john-bodley Updated both 


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 issue #4836: Refactoring on exploreReducer.js

[GitBox]

hug commented on issue #4836: Refactoring on exploreReducer.js
URL: 
https://github.com/apache/incubator-superset/pull/4836#issuecomment-382525145
 
 
   I was following the same pattern for the Object.assign which is merging 
state and controls together. I've refactored it using the spread operator 
notation.
   ```
   > a
   { foo: 'bar' }
   > b
   { hugh: 'miles' }
   > {...a, ...b}
   { foo: 'bar', hugh: 'miles' }
   ```


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 issue #4836: Refactoring on exploreReducer.js

[GitBox]

hug commented on issue #4836: Refactoring on exploreReducer.js
URL: 
https://github.com/apache/incubator-superset/pull/4836#issuecomment-382525145
 
 
   I was following the same pattern for the Object.assign which is merging 
state and controls together. I've refactored it using the spread operator 
notation.
   
   > a
   { foo: 'bar' }
   > b
   { hugh: 'miles' }
   > {...a, ...b}
   { foo: 'bar', hugh: 'miles' }


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 #4836: Refactoring on exploreReducer.js

[GitBox]

hug commented on a change in pull request #4836: Refactoring on 
exploreReducer.js
URL: 
https://github.com/apache/incubator-superset/pull/4836#discussion_r182546092
 
 

 ##
 File path: superset/assets/javascripts/explore/reducers/exploreReducer.js
 ##
 @@ -5,77 +5,118 @@ import * as actions from '../actions/exploreActions';
 export default function exploreReducer(state = {}, action) {
   const actionHandlers = {
 [actions.TOGGLE_FAVE_STAR]() {
-  return Object.assign({}, state, { isStarred: action.isStarred });
+  return {
+...state,
+isStarred: action.isStarred,
+  };
 },
 [actions.FETCH_DATASOURCE_STARTED]() {
-  return Object.assign({}, state, { isDatasourceMetaLoading: true });
+  return {
+...state,
+isDatasourceMetaLoading: true,
+  };
 },
 [actions.FETCH_DATASOURCE_SUCCEEDED]() {
-  return Object.assign({}, state, { isDatasourceMetaLoading: false });
+  return {
+...state,
+isDatasourceMetaLoading: false,
+  };
 },
 [actions.FETCH_DATASOURCE_FAILED]() {
-  // todo(alanna) handle failure/error state
-  return Object.assign({}, state,
-{
-  isDatasourceMetaLoading: false,
-  controlPanelAlert: action.error,
-});
+  return {
+...state,
+isDatasourceMetaLoading: false,
+controlPanelAlert: action.error,
+  };
 },
 [actions.SET_DATASOURCE]() {
-  return Object.assign({}, state, { datasource: action.datasource });
+
+  return {
+...state,
+datasource: action.datasource,
+  };
 },
 [actions.FETCH_DATASOURCES_STARTED]() {
-  return Object.assign({}, state, { isDatasourcesLoading: true });
+
+  return {
+...state,
+isDatasourcesLoading: true,
+  };
 },
 [actions.FETCH_DATASOURCES_SUCCEEDED]() {
-  return Object.assign({}, state, { isDatasourcesLoading: false });
+
+  return {
+...state,
+isDatasourcesLoading: false,
+  };
 },
 [actions.FETCH_DATASOURCES_FAILED]() {
-  // todo(alanna) handle failure/error state
-  return Object.assign({}, state,
-{
-  isDatasourcesLoading: false,
-  controlPanelAlert: action.error,
-});
+
+  return {
+...state,
+isDatasourcesLoading: false,
+controlPanelAlert: action.error,
+  };
 },
 [actions.SET_DATASOURCES]() {
-  return Object.assign({}, state, { datasources: action.datasources });
+  return {
+...state,
+datasources: action.datasources,
+  };
 },
 [actions.REMOVE_CONTROL_PANEL_ALERT]() {
-  return Object.assign({}, state, { controlPanelAlert: null });
+  return {
+...state,
+controlPanelAlert: null,
+  };
 },
 [actions.SET_FIELD_VALUE]() {
   const controls = Object.assign({}, state.controls);
   const control = Object.assign({}, controls[action.controlName]);
   control.value = action.value;
   control.validationErrors = action.validationErrors;
   controls[action.controlName] = control;
-  const changes = { controls };
+  const changes = {
+controls,
+  };
   if (control.renderTrigger) {
 changes.triggerRender = true;
   }
-  return Object.assign({}, state, changes);
+  return {
+...state,
+...changes,
+  };
 },
 [actions.SET_EXPLORE_CONTROLS]() {
   const controls = getControlsState(state, action.formData);
-  return Object.assign({}, state, { controls });
+  return {
+...state,
+...controls,
+  };
 },
 [actions.UPDATE_CHART_TITLE]() {
   const updatedSlice = Object.assign({}, state.slice, { slice_name: 
action.slice_name });
-  return Object.assign({}, state, { slice: updatedSlice });
+  return {
+...state,
+slice: updatedSlice,
+  };
 },
 [actions.RESET_FIELDS]() {
   const controls = getControlsState(state, 
getFormDataFromControls(state.controls));
-  return Object.assign({}, state, { controls });
+  return {
+...state,
+...controls,
 
 Review comment:
   I was following the same pattern for the `Object.assign` which is merging 
state and controls together. I've refactored it using the spread operator 
notation.
   ```
   > a
   { foo: 'bar' }
   > b
   { hugh: 'miles' }
   > {...a, ...b}
   { foo: 'bar', hugh: 'miles' }
   ```


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] abowden-spotx commented on issue #4747: [WiP] Download Dashboards as PDF

[GitBox]

abowden-spotx commented on issue #4747: [WiP] Download Dashboards as PDF
URL: 
https://github.com/apache/incubator-superset/pull/4747#issuecomment-382427457
 
 
   Ultimately, the goal of this addition is so that we can integrate it with an 
emailing feature that will run independently on the server, which will allow us 
to send visualizations of the data to our ops teams.
   
   So far, I have looked into:
   
   - [pdfkit](https://pypi.org/project/pdfkit/) (Python wrapper for 
[wkhtmltopdf](https://wkhtmltopdf.org/))
   - [jsPDF](https://github.com/MrRio/jsPDF) 
   
   pdfkit seems to be the best option since it will allow us to download the 
dash directly from the server. The main hurdle to get over is appropriately 
passing the user session for authentication.
   
   Current process is as follows:
   
   - Render 'Download' button in the dashboard header 
(/javascripts/dashboard/components/Header.jsx)
   - OnClick make AJAX request (/javascripts/reduxUtils.js)
   - pass authentication/url/etc to server at specified route (/views/core.py)
   - call pdfkit.from_url() to generate PDF from provided arguments


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 #4836: [WIP] Refactoring on exploreReducer.js

[GitBox]

codecov-io commented on issue #4836: [WIP] Refactoring on exploreReducer.js
URL: 
https://github.com/apache/incubator-superset/pull/4836#issuecomment-382512749
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4836?src=pr=h1)
 Report
   > Merging 
[#4836](https://codecov.io/gh/apache/incubator-superset/pull/4836?src=pr=desc)
 into 
[master](https://codecov.io/gh/apache/incubator-superset/commit/eac97ce9f201be1c8245aa9ad452c05f702f8fd3?src=pr=desc)
 will **not change** coverage.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-superset/pull/4836/graphs/tree.svg?width=650=150=pr=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/4836?src=pr=tree)
   
   ```diff
   @@   Coverage Diff   @@
   ##   master#4836   +/-   ##
   ===
 Coverage   76.91%   76.91%   
   ===
 Files  44   44   
 Lines8522 8522   
   ===
 Hits 6555 6555   
 Misses   1967 1967
   ```
   
   
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4836?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/4836?src=pr=footer).
 Last update 
[eac97ce...aaf43cd](https://codecov.io/gh/apache/incubator-superset/pull/4836?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 #4829: [import csv] ensure directory exists before saving csv file

[GitBox]

codecov-io commented on issue #4829: [import csv] ensure directory exists 
before saving csv file
URL: 
https://github.com/apache/incubator-superset/pull/4829#issuecomment-381390257
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4829?src=pr=h1)
 Report
   > Merging 
[#4829](https://codecov.io/gh/apache/incubator-superset/pull/4829?src=pr=desc)
 into 
[master](https://codecov.io/gh/apache/incubator-superset/commit/eac97ce9f201be1c8245aa9ad452c05f702f8fd3?src=pr=desc)
 will **increase** coverage by `0.01%`.
   > The diff coverage is `83.33%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-superset/pull/4829/graphs/tree.svg?width=650=pr=KsB0fHcx6l=150)](https://codecov.io/gh/apache/incubator-superset/pull/4829?src=pr=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master#4829  +/-   ##
   ==
   + Coverage   76.91%   76.93%   +0.01% 
   ==
 Files  44   44  
 Lines8522 8531   +9 
   ==
   + Hits 6555 6563   +8 
   - Misses   1967 1968   +1
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-superset/pull/4829?src=pr=tree)
 | Coverage Δ | |
   |---|---|---|
   | 
[superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/4829/diff?src=pr=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==)
 | `74.68% <80%> (+0.03%)` | :arrow_up: |
   | 
[superset/utils.py](https://codecov.io/gh/apache/incubator-superset/pull/4829/diff?src=pr=tree#diff-c3VwZXJzZXQvdXRpbHMucHk=)
 | `88.06% <85.71%> (-0.04%)` | :arrow_down: |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4829?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/4829?src=pr=footer).
 Last update 
[eac97ce...7192327](https://codecov.io/gh/apache/incubator-superset/pull/4829?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] hughhhh commented on a change in pull request #4836: [WIP] Refactoring on exploreReducer.js

[GitBox]

hug commented on a change in pull request #4836: [WIP] Refactoring on 
exploreReducer.js
URL: 
https://github.com/apache/incubator-superset/pull/4836#discussion_r182546092
 
 

 ##
 File path: superset/assets/javascripts/explore/reducers/exploreReducer.js
 ##
 @@ -5,77 +5,118 @@ import * as actions from '../actions/exploreActions';
 export default function exploreReducer(state = {}, action) {
   const actionHandlers = {
 [actions.TOGGLE_FAVE_STAR]() {
-  return Object.assign({}, state, { isStarred: action.isStarred });
+  return {
+...state,
+isStarred: action.isStarred,
+  };
 },
 [actions.FETCH_DATASOURCE_STARTED]() {
-  return Object.assign({}, state, { isDatasourceMetaLoading: true });
+  return {
+...state,
+isDatasourceMetaLoading: true,
+  };
 },
 [actions.FETCH_DATASOURCE_SUCCEEDED]() {
-  return Object.assign({}, state, { isDatasourceMetaLoading: false });
+  return {
+...state,
+isDatasourceMetaLoading: false,
+  };
 },
 [actions.FETCH_DATASOURCE_FAILED]() {
-  // todo(alanna) handle failure/error state
-  return Object.assign({}, state,
-{
-  isDatasourceMetaLoading: false,
-  controlPanelAlert: action.error,
-});
+  return {
+...state,
+isDatasourceMetaLoading: false,
+controlPanelAlert: action.error,
+  };
 },
 [actions.SET_DATASOURCE]() {
-  return Object.assign({}, state, { datasource: action.datasource });
+
+  return {
+...state,
+datasource: action.datasource,
+  };
 },
 [actions.FETCH_DATASOURCES_STARTED]() {
-  return Object.assign({}, state, { isDatasourcesLoading: true });
+
+  return {
+...state,
+isDatasourcesLoading: true,
+  };
 },
 [actions.FETCH_DATASOURCES_SUCCEEDED]() {
-  return Object.assign({}, state, { isDatasourcesLoading: false });
+
+  return {
+...state,
+isDatasourcesLoading: false,
+  };
 },
 [actions.FETCH_DATASOURCES_FAILED]() {
-  // todo(alanna) handle failure/error state
-  return Object.assign({}, state,
-{
-  isDatasourcesLoading: false,
-  controlPanelAlert: action.error,
-});
+
+  return {
+...state,
+isDatasourcesLoading: false,
+controlPanelAlert: action.error,
+  };
 },
 [actions.SET_DATASOURCES]() {
-  return Object.assign({}, state, { datasources: action.datasources });
+  return {
+...state,
+datasources: action.datasources,
+  };
 },
 [actions.REMOVE_CONTROL_PANEL_ALERT]() {
-  return Object.assign({}, state, { controlPanelAlert: null });
+  return {
+...state,
+controlPanelAlert: null,
+  };
 },
 [actions.SET_FIELD_VALUE]() {
   const controls = Object.assign({}, state.controls);
   const control = Object.assign({}, controls[action.controlName]);
   control.value = action.value;
   control.validationErrors = action.validationErrors;
   controls[action.controlName] = control;
-  const changes = { controls };
+  const changes = {
+controls,
+  };
   if (control.renderTrigger) {
 changes.triggerRender = true;
   }
-  return Object.assign({}, state, changes);
+  return {
+...state,
+...changes,
+  };
 },
 [actions.SET_EXPLORE_CONTROLS]() {
   const controls = getControlsState(state, action.formData);
-  return Object.assign({}, state, { controls });
+  return {
+...state,
+...controls,
+  };
 },
 [actions.UPDATE_CHART_TITLE]() {
   const updatedSlice = Object.assign({}, state.slice, { slice_name: 
action.slice_name });
-  return Object.assign({}, state, { slice: updatedSlice });
+  return {
+...state,
+slice: updatedSlice,
+  };
 },
 [actions.RESET_FIELDS]() {
   const controls = getControlsState(state, 
getFormDataFromControls(state.controls));
-  return Object.assign({}, state, { controls });
+  return {
+...state,
+...controls,
 
 Review comment:
   I was following the same pattern for the `Object.assign` which is merging 
state and controls together. I've refactored it using the spread operator 
notation.
   ```
   > a
   { foo: 'bar' }
   > b
   { hugh: 'miles' }
   > {...a, ...b}
   { foo: 'bar', hugh: 'miles' }
   ```


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] deshpand commented on issue #3312: Proposal: make single sign on and other auth/authn easier by removing Flask AppBuilder

[GitBox]

deshpand commented on issue #3312: Proposal: make single sign on and other 
auth/authn easier by removing Flask AppBuilder
URL: 
https://github.com/apache/incubator-superset/issues/3312#issuecomment-382481725
 
 
   I have been researching both superset and airflow for adoption at my 
enterprise (thanks mistercrunch for your open source contributions!) and I know 
airflow is also planning to use FAB for security.  I am just beginning to look 
at the security aspects and I have some similar thoughts/needs as shared by 
laranzu.
   
   The gist code was very helpful to get me started but I may not be able to 
have a database at the backend, like FAB seems to presume.  
   
   I currently have a flask based application where I get a SamlToken from SSO 
provider which gives me user id, AD groups the user belongs to.  From the AD 
groups, I construct roles (that I persist in session/cookie) and use Flask 
Principal and provide role based access to routes.  I am hoping for a similar 
mechanism with a superset_config where I override the bare minimum classes.
   
   I hope I am at least making sense!  Please do let me know of additional 
resources, possibly including any work by laranzu that he may want to share.


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] deshpand commented on issue #3312: Proposal: make single sign on and other auth/authn easier by removing Flask AppBuilder

[GitBox]

deshpand commented on issue #3312: Proposal: make single sign on and other 
auth/authn easier by removing Flask AppBuilder
URL: 
https://github.com/apache/incubator-superset/issues/3312#issuecomment-382481725
 
 
   I have researching both superset and airflow for adoption at my enterprise 
(thanks mistercrunch for your open source contributions!) and I know airflow is 
also planning to use FAB for security.  I am just beginning to look at the 
security aspects and I have some similar thoughts/needs as shared by laranzu.
   
   The gist code was very helpful to get me started but I may not be able to 
have a database at the backend, like FAB seems to presume.  
   
   I currently have a flask based application where I get a SamlToken from SSO 
provider which gives me user id, AD groups the user belongs to.  From the AD 
groups, I construct roles (that I persist in session/cookie) and use Flask 
Principal and provide role based access to routes.  I am hoping for a similar 
mechanism with a superset_config where I override the bare minimum classes.
   
   I hope I am at least making sense!  Please do let me know of additional 
resources, possibly including any work by laranzu that he may want to share.


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 #4834: [WIP] Load async sql lab results early for Presto

[GitBox]

john-bodley commented on issue #4834: [WIP] Load async sql lab results early 
for Presto
URL: 
https://github.com/apache/incubator-superset/pull/4834#issuecomment-382476044
 
 
   @timifasubaa if you update your description the following [keywords]( 
https://help.github.com/articles/closing-issues-using-keywords/) can be use to 
automatically close issues.


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 #4834: [WIP] Load async sql lab results early for Presto

[GitBox]

john-bodley commented on issue #4834: [WIP] Load async sql lab results early 
for Presto
URL: 
https://github.com/apache/incubator-superset/pull/4834#issuecomment-382475362
 
 
   @timifasubaa would you mind including screenshots and/or an animated gif 
(you can use LICEcap to record a video) to provide more context with regards to 
how the UX is changed.


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 #4834: [WIP] Load async sql lab results early for Presto

[GitBox]

john-bodley commented on issue #4834: [WIP] Load async sql lab results early 
for Presto
URL: 
https://github.com/apache/incubator-superset/pull/4834#issuecomment-382475362
 
 
   @timifasubaa would you mind including screenshots and/or an animated gif 
(you can use LICEcap to record a video) to provide more context with regards to 
how the UX has changed.


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 #4833: [sqllab] Help sqllab forget query history

[GitBox]

john-bodley commented on issue #4833: [sqllab] Help sqllab forget query history
URL: 
https://github.com/apache/incubator-superset/pull/4833#issuecomment-382474470
 
 
   I agree with @mistercrunch it would be great to change the state in the 
`queries` table for clarity and auditing purposes. Note this would actually be 
a backend change as opposed to a frontend change.


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 #4829: [import csv] ensure directory exists before saving csv file

[GitBox]

john-bodley commented on issue #4829: [import csv] ensure directory exists 
before saving csv file
URL: 
https://github.com/apache/incubator-superset/pull/4829#issuecomment-382473280
 
 
   @timifasubaa if you rebase your change the CI should succeed.


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 #4520: Allow users to view dashboards they own

[GitBox]

codecov-io commented on issue #4520: Allow users to view dashboards they own
URL: 
https://github.com/apache/incubator-superset/pull/4520#issuecomment-382471867
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4520?src=pr=h1)
 Report
   > Merging 
[#4520](https://codecov.io/gh/apache/incubator-superset/pull/4520?src=pr=desc)
 into 
[master](https://codecov.io/gh/apache/incubator-superset/commit/eac97ce9f201be1c8245aa9ad452c05f702f8fd3?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/4520/graphs/tree.svg?src=pr=KsB0fHcx6l=650=150)](https://codecov.io/gh/apache/incubator-superset/pull/4520?src=pr=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master#4520  +/-   ##
   ==
   + Coverage   76.91%   76.92%   +<.01% 
   ==
 Files  44   44  
 Lines8522 8524   +2 
   ==
   + Hits 6555 6557   +2 
 Misses   1967 1967
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-superset/pull/4520?src=pr=tree)
 | Coverage Δ | |
   |---|---|---|
   | 
[superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/4520/diff?src=pr=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==)
 | `74.68% <100%> (+0.03%)` | :arrow_up: |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-superset/pull/4520?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/4520?src=pr=footer).
 Last update 
[eac97ce...513ceb0](https://codecov.io/gh/apache/incubator-superset/pull/4520?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

Re: Setting up SLAs at DAG and Job level

[Yogesh Kumar]

my bad. apologize for inconvenience.

On Wed, Apr 18, 2018 at 8:25 AM, Maxime Beauchemin <
maximebeauche...@gmail.com> wrote:

> This is the Superset mailing list, not the Airflow one...
>
> On Tue, Apr 17, 2018 at 5:53 PM, Yogesh Kumar 
> wrote:
>
> > Hi Devs,
> >  Is there any way to do that based on actual start-date of DAG instance
> > rather than based on execution_date?  I have multiple scenarios of
> > backfilling data and setting up SLA triggers alerts immediately which is
> > annoying.
> >
> > Any pointers?
> >
> > Thanks,
> > -Y
> >
>

[GitHub] mistercrunch opened a new pull request #4841: Move a few JS files

[GitBox]

mistercrunch opened a new pull request #4841: Move a few JS files
URL: https://github.com/apache/incubator-superset/pull/4841
 
 
   `src/explore/stores/store` - > `src/explore/store`
   `src/explore/stores/visTypes` -> `src/explore/visTypes`
   `src/visualizations/main.js` -> `src/visualizations/index.js`


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 #4205: Ability to save default values to Dashboard filter

[GitBox]

mistercrunch commented on issue #4205: Ability to save default values to 
Dashboard filter
URL: 
https://github.com/apache/incubator-superset/issues/4205#issuecomment-382434755
 
 
   @sevalo8 are you using an old version?


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 #4840: ldap configuration

[GitBox]

mistercrunch commented on issue #4840: ldap configuration
URL: 
https://github.com/apache/incubator-superset/issues/4840#issuecomment-382434085
 
 
   This is really a FAB (Flask-AppBuilder) question.


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] abowden-spotx commented on issue #4747: [WiP] Download Dashboards as PDF

[GitBox]

abowden-spotx commented on issue #4747: [WiP] Download Dashboards as PDF
URL: 
https://github.com/apache/incubator-superset/pull/4747#issuecomment-382427457
 
 
   Ultimately, the goal of this addition is so that we can integrate it with an 
emailing feature that will run independently on the server, which will allow us 
to send visualizations of the data to our ops teams.
   
   So far, I have looked into:
   
   - [pdfkit](https://pypi.org/project/pdfkit/) (Python wrapper for 
[wkhtmltopdf](https://wkhtmltopdf.org/))
   - [jsPDF](https://github.com/MrRio/jsPDF) 
   
   pdfkit seems to be the best option since it will allow us to download the 
dash directly from the server. The main hurdle to get over is appropriately 
passing the user session for authentication.


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] abowden-spotx commented on issue #4747: [WiP] Download Dashboards as PDF

[GitBox]

abowden-spotx commented on issue #4747: [WiP] Download Dashboards as PDF
URL: 
https://github.com/apache/incubator-superset/pull/4747#issuecomment-382427457
 
 
   Ultimately, the goal of this addition is so that we can integrate it with an 
emailing feature that will run independently on the server, which will allow us 
to send visualizations of the data to our ops teams.
   
   So far, I have looked into:
   
   - [pdfkit](https://pypi.org/project/pdfkit/) (Python wrapper for 
[wkhtmltopdf](https://wkhtmltopdf.org/))
   - [jsPDF](https://github.com/MrRio/jsPDF) 
   


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

Re: Setting up SLAs at DAG and Job level

[Maxime Beauchemin]

This is the Superset mailing list, not the Airflow one...

On Tue, Apr 17, 2018 at 5:53 PM, Yogesh Kumar 
wrote:

> Hi Devs,
>  Is there any way to do that based on actual start-date of DAG instance
> rather than based on execution_date?  I have multiple scenarios of
> backfilling data and setting up SLA triggers alerts immediately which is
> annoying.
>
> Any pointers?
>
> Thanks,
> -Y
>

[GitHub] qin4zhang opened a new issue #4840: ldap configuration

[GitBox]

qin4zhang opened a new issue #4840: ldap configuration
URL: https://github.com/apache/incubator-superset/issues/4840
 
 
   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.22.1
   
   ### Expected results
   login successfully
   
   ### Actual results
   cannot login in
   
   ### Steps to reproduce
   modify config.py
   from flask_appbuilder.security.manager import AUTH_LDAP
   AUTH_TYPE = AUTH_LDAP
   AUTH_LDAP_SERVER = "ldap://server:port;
   AUTH_LDAP_USE_TLS = False
   AUTH_LDAP_SEARCH = "dc=**"
   AUTH_LDAP_BIND_USER = "uid=**,ou=**,dc=**,dc=**"
   AUTH_LDAP_BIND_PASSWORD = "**"
   AUTH_LDAP_UID_FIELD = "sAMAccountName"
   AUTH_LDAP_APPEND_DOMAIN = '**'
   
   console in debug mode:
   -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
   Starting Superset server in DEBUG mode
   -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
   
   2018-04-18 19:01:31,195:WARNING:werkzeug: * Debugger is active!
   2018-04-18 19:01:31,195:INFO:werkzeug: * Debugger PIN: 859-227-507
   2018-04-18 19:02:03,228:INFO:werkzeug:172.22.4.100 - - [18/Apr/2018 
19:02:03] "GET /login/ HTTP/1.1" 200 -
   2018-04-18 19:02:04,699:INFO:werkzeug:172.22.4.100 - - [18/Apr/2018 
19:02:04] "GET /static/appbuilder/css/bootstrap.min.css.map HTTP/1.1" 404 -
   2018-04-18 19:02:11,270:INFO:werkzeug:172.22.4.100 - - [18/Apr/2018 
19:02:11] "POST /login/ HTTP/1.1" 302 -
   2018-04-18 19:02:11,420:INFO:werkzeug:172.22.4.100 - - [18/Apr/2018 
19:02:11] "GET /login/ HTTP/1.1" 200 -
   2018-04-18 19:02:12,952:INFO:werkzeug:172.22.4.100 - - [18/Apr/2018 
19:02:12] "GET /static/appbuilder/css/bootstrap.min.css.map HTTP/1.1" 404 -
   2018-04-18 19:02:25,235:INFO:werkzeug:172.22.4.100 - - [18/Apr/2018 
19:02:25] "POST /login/ HTTP/1.1" 302 -
   2018-04-18 19:02:25,396:INFO:werkzeug:172.22.4.100 - - [18/Apr/2018 
19:02:25] "GET /login/ HTTP/1.1" 200 -
   2018-04-18 19:02:26,905:INFO:werkzeug:172.22.4.100 - - [18/Apr/2018 
19:02:26] "GET /static/appbuilder/css/bootstrap.min.css.map HTTP/1.1" 404 -
   
   however, I cannot login in. LDAP log missed.How could I do to solve 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] sevalo8 commented on issue #4205: Ability to save default values to Dashboard filter

[GitBox]

sevalo8 commented on issue #4205: Ability to save default values to Dashboard 
filter
URL: 
https://github.com/apache/incubator-superset/issues/4205#issuecomment-382323853
 
 
   Hi @graceguo-supercat , 
   i try as the animation that you share, but why the filter still not set as 
when i klik di save.
   when i open the dashboard it's always show blank filter.
   Anything that i need to set before?
   
   Thanks,
   
   
   Steven


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