You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/04/21 18:07:23 UTC

[GitHub] [superset] cemremengu opened a new pull request, #19810: feat(reports): xlsx attachments

cemremengu opened a new pull request, #19810:
URL: https://github.com/apache/superset/pull/19810

   ### SUMMARY
   
   Most people and departments in enterprise companies are addicted to spreadsheets. Currently our customers are heavily demanding Excel reports and, unfortunately, we have failed to persuade them to continue with CSV.
   
   So here I am with a PR that adds the ability to attach XLSX files as report data and notifications (slack). If this one gets approved, I will also attempt to implement PDF in a similar way.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   ![image](https://user-images.githubusercontent.com/1297759/164520812-b36c4285-5a03-4f72-903d-646db17b43d9.png)
   
   ![image](https://user-images.githubusercontent.com/1297759/164520865-d877faa7-a245-4179-bb1d-3d556a0c16c0.png)
   
   ### TESTING INSTRUCTIONS
   Try sending an xlsx report or notification. I wasn't able to test slack since I don't use it actively but implemented it nevertheless for completeness.
   
   ### ADDITIONAL INFORMATION
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [x] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [x] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] EugeneTorap commented on pull request #19810: feat(reports): xlsx attachments

Posted by GitBox <gi...@apache.org>.
EugeneTorap commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1339155386

   @cemremengu We use `Excel` name instead of `XLSX` in our https://github.com/apache/superset/pull/22006 PR
   Can we use the `Excel` name for email alert/report UI logic? 
   
   ![image](https://user-images.githubusercontent.com/29536522/205895111-49fe7e0c-ed11-4107-80dc-8b19518ad575.png)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] mdeshmu commented on pull request #19810: feat(reports): xlsx attachments

Posted by "mdeshmu (via GitHub)" <gi...@apache.org>.
mdeshmu commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1553108152

   @cemremengu did you get time to fix this ?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] cemremengu commented on pull request #19810: feat(reports): xlsx attachments

Posted by GitBox <gi...@apache.org>.
cemremengu commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1246949125

   You are welcome! I am happy to continue once maintainers have time to chime in.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] cemremengu commented on pull request #19810: feat(reports): xlsx attachments

Posted by GitBox <gi...@apache.org>.
cemremengu commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1339169297

   Just to clarify @EugeneTorap should something like this be
   
   `<StyledRadio value="EXCEL">{t('Send as Excel')}</StyledRadio>`
   
   or
   
   `<StyledRadio value="Excel">{t('Send as Excel')}</StyledRadio>`


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] EugeneTorap commented on a diff in pull request #19810: feat(reports): xlsx attachments

Posted by "EugeneTorap (via GitHub)" <gi...@apache.org>.
EugeneTorap commented on code in PR #19810:
URL: https://github.com/apache/superset/pull/19810#discussion_r1089943052


##########
superset/reports/commands/execute.py:
##########
@@ -352,11 +366,18 @@ def _get_notification_content(self) -> NotificationContent:
                     error_text = "Unexpected missing screenshot"
             elif (
                 self._report_schedule.chart
-                and self._report_schedule.report_format == ReportDataFormat.DATA
+                and self._report_schedule.report_format == ReportDataFormat.CSV

Review Comment:
   Add `table_like` method for `ReportDataFormat` and use the next condition:
   ```python
   if self._report_schedule.report_format in ReportDataFormat.table_like():
   ```
   
   For `ReportDataFormat`:
   ```python
   @classmethod
   def table_like(cls) -> Set["ReportDataFormat"]:
       return {cls.CSV} | {cls.XLSX}
   ```



##########
superset/reports/commands/execute.py:
##########
@@ -352,11 +366,18 @@ def _get_notification_content(self) -> NotificationContent:
                     error_text = "Unexpected missing screenshot"
             elif (
                 self._report_schedule.chart
-                and self._report_schedule.report_format == ReportDataFormat.DATA
+                and self._report_schedule.report_format == ReportDataFormat.CSV
             ):
-                csv_data = self._get_csv_data()
-                if not csv_data:
+                data = self._get_csv_data()

Review Comment:
   Rename `self._get_csv_data()` to `self._get_data()` in order to use CSV and Excel logic in one method.
   I think it's better to use `self._get_data()` instead of `self._get_csv_data()` and `self._get_xlsx_data()` because we don't write the boilerplate code. If we want to add a data format for example XML, we do it in one `self._get_data()`.
   I think we can use `get_chart_dataframe()` func for CSV and Excel. This func returns `df` which we can use it in `excel.df_to_excel()`.
   I want to reuse the Chart Export func instead of having a lot of the same code for chart & report data exporting.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] EugeneTorap commented on pull request #19810: feat(reports): xlsx attachments

Posted by GitBox <gi...@apache.org>.
EugeneTorap commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1339172660

   @cemremengu Value must be `XLSX`
   ```
   <StyledRadio value="XLSX">{t('Send as Excel')}</StyledRadio>
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] cemremengu commented on pull request #19810: feat(reports): xlsx attachments

Posted by "cemremengu (via GitHub)" <gi...@apache.org>.
cemremengu commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1407679698

   @EugeneTorap here is my attempt, still needs to be checked for runtime errors in a test-env though.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] artemonsh commented on pull request #19810: feat(reports): xlsx attachments

Posted by GitBox <gi...@apache.org>.
artemonsh commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1308516388

   @cemremengu you forgot to add 
   ![image](https://user-images.githubusercontent.com/53895552/200802290-73777dbb-6fbe-44c8-9be3-4cc25d001150.png)
   
   
   to the file superset/reports/commands/execute.py
   
   Otherwise, email reports look like this:
   ![image](https://user-images.githubusercontent.com/53895552/200802472-e047287a-9693-4543-83fe-9dd2d7f6beed.png)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [PR] feat(reports): xlsx attachments [superset]

Posted by "mdeshmu (via GitHub)" <gi...@apache.org>.
mdeshmu commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1747867136

   @rusackas can we please unblock this by answering @cemremengu's question in last comment.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] cemremengu commented on pull request #19810: feat(reports): xlsx attachments

Posted by "cemremengu (via GitHub)" <gi...@apache.org>.
cemremengu commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1424315145

   @kakoni thank you for the tip!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] EugeneTorap commented on a diff in pull request #19810: feat(reports): xlsx attachments

Posted by "EugeneTorap (via GitHub)" <gi...@apache.org>.
EugeneTorap commented on code in PR #19810:
URL: https://github.com/apache/superset/pull/19810#discussion_r1089949385


##########
superset/reports/commands/execute.py:
##########
@@ -352,11 +366,18 @@ def _get_notification_content(self) -> NotificationContent:
                     error_text = "Unexpected missing screenshot"
             elif (
                 self._report_schedule.chart
-                and self._report_schedule.report_format == ReportDataFormat.DATA
+                and self._report_schedule.report_format == ReportDataFormat.CSV
             ):
-                csv_data = self._get_csv_data()
-                if not csv_data:
+                data = self._get_csv_data()

Review Comment:
   Is it possible to create one `def get_data(self, df: pd.DataFrame)` method for chart and report?
   It will be better to maintain the code in the future.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] rusackas commented on pull request #19810: feat(reports): xlsx attachments

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1591512760

   The `test-mysql`, `test-postgres`, and `test-sqlite` actions are failing due to this:
   > FAILED tests/integration_tests/reports/commands_tests.py::test_email_chart_report_schedule_with_csv - superset.reports.commands.exceptions.ReportScheduleDataFrameFailedError: Failed generating dataframe Input string must be text, not bytes
   
   the `pre-commit` check is failing because of a linting error. You can probably just run `pre-commit run --all-files` and commit any changes to clear that up.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] etadelta222 commented on pull request #19810: feat(reports): xlsx attachments

Posted by GitBox <gi...@apache.org>.
etadelta222 commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1245903804

   @cemremengu , I've posted this in superset slack (https://apache-superset.slack.com/archives/CCKHMGRRB/p1662748936923259) and there's some feedback. Hopefully get some 👀 on it. Thank you for creating this PR!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] kakoni commented on pull request #19810: feat(reports): xlsx attachments

Posted by "kakoni (via GitHub)" <gi...@apache.org>.
kakoni commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1419081134

   @cemremengu Btw, python integration tests are currently failing for this, 
   ```superset.reports.commands.exceptions.ReportScheduleUnexpectedError: argument of type 'method' is not iterable```
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] cemremengu commented on pull request #19810: feat(reports): xlsx attachments

Posted by "cemremengu (via GitHub)" <gi...@apache.org>.
cemremengu commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1584239134

   Not sure what I need to fix.  As far as I remember failing tests were flaky (I cannot see the CI log anymore)
   
   @EugeneTorap @rusackas how should we proceed with this PR?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] EugeneTorap commented on pull request #19810: feat(reports): xlsx attachments

Posted by "EugeneTorap (via GitHub)" <gi...@apache.org>.
EugeneTorap commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1399946671

   Hi @dpgaspar @villebro. Can you review the PR?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] cemremengu commented on pull request #19810: feat(reports): xlsx attachments

Posted by GitBox <gi...@apache.org>.
cemremengu commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1339176539

   @EugeneTorap Fixed 2 of them, I agree it is better like this.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] EugeneTorap commented on a diff in pull request #19810: feat(reports): xlsx attachments

Posted by "EugeneTorap (via GitHub)" <gi...@apache.org>.
EugeneTorap commented on code in PR #19810:
URL: https://github.com/apache/superset/pull/19810#discussion_r1089720796


##########
superset/reports/commands/execute.py:
##########
@@ -330,13 +332,25 @@ def _get_log_data(self) -> HeaderDataType:
         }
         return log_data
 
+    def _get_xlsx_data(self) -> bytes:
+        csv_data = self._get_csv_data()
+
+        df = pd.read_csv(BytesIO(csv_data))
+        bio = BytesIO()
+
+        # pylint: disable=abstract-class-instantiated
+        with pd.ExcelWriter(bio, engine="openpyxl") as writer:
+            df.to_excel(writer, index=False)

Review Comment:
   Use `df_to_excel` in `superset/utils/excel.py`



##########
superset/reports/commands/execute.py:
##########
@@ -330,13 +332,25 @@ def _get_log_data(self) -> HeaderDataType:
         }
         return log_data
 
+    def _get_xlsx_data(self) -> bytes:
+        csv_data = self._get_csv_data()
+
+        df = pd.read_csv(BytesIO(csv_data))

Review Comment:
   Retrieving `df` from csv data it's not the good approach. We need to write a common method for csv and excel where we provide `df` param like `get_data(self, df: pd.DataFrame)` in `superset/common/query_context_processor.py`.
   @dpgaspar Am I correct?
   ![image](https://user-images.githubusercontent.com/29536522/215264995-1cfd07c4-1c12-43b8-8d03-2920b40d0ce1.png)
   



##########
superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx:
##########
@@ -418,6 +418,7 @@ const TRANSLATIONS = {
   CHART_TEXT: t('Chart'),
   SEND_AS_PNG_TEXT: t('Send as PNG'),
   SEND_AS_CSV_TEXT: t('Send as CSV'),
+  SEND_AS_XLSX_TEXT: t('Send as XLSX'),

Review Comment:
   Rename to 'Send as Excel'



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] cemremengu commented on pull request #19810: feat(reports): xlsx attachments

Posted by "cemremengu (via GitHub)" <gi...@apache.org>.
cemremengu commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1590782983

   @rusackas rebased, not sure what the failing tests are about. 
   
   @kakoni had doubts about `_get_data ` vs `_get_csv_data`. Once we settle that I think we can merge?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] codecov[bot] commented on pull request #19810: feat(reports): xlsx attachments

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1105595530

   # [Codecov](https://codecov.io/gh/apache/superset/pull/19810?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#19810](https://codecov.io/gh/apache/superset/pull/19810?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d28fb6b) into [master](https://codecov.io/gh/apache/superset/commit/3db4a1cb8016dae71b5bfca09ef723c042dff671?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3db4a1c) will **decrease** coverage by `12.60%`.
   > The diff coverage is `26.08%`.
   
   > :exclamation: Current head d28fb6b differs from pull request most recent head 658760b. Consider uploading reports for the commit 658760b to get more accurate results
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #19810       +/-   ##
   ===========================================
   - Coverage   66.54%   53.94%   -12.61%     
   ===========================================
     Files        1692     1692               
     Lines       64807    64818       +11     
     Branches     6661     6661               
   ===========================================
   - Hits        43129    34964     -8165     
   - Misses      19978    28154     +8176     
     Partials     1700     1700               
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `52.86% <22.72%> (-0.01%)` | :arrow_down: |
   | mysql | `?` | |
   | postgres | `?` | |
   | presto | `52.71% <22.72%> (-0.01%)` | :arrow_down: |
   | python | `56.72% <22.72%> (-25.65%)` | :arrow_down: |
   | sqlite | `?` | |
   | unit | `47.94% <9.09%> (-0.02%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/19810?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...rset-frontend/src/components/ReportModal/index.tsx](https://codecov.io/gh/apache/superset/pull/19810/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvUmVwb3J0TW9kYWwvaW5kZXgudHN4) | `78.46% <ø> (ø)` | |
   | [superset/reports/notifications/email.py](https://codecov.io/gh/apache/superset/pull/19810/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvcmVwb3J0cy9ub3RpZmljYXRpb25zL2VtYWlsLnB5) | `48.48% <0.00%> (-51.52%)` | :arrow_down: |
   | [superset/reports/notifications/slack.py](https://codecov.io/gh/apache/superset/pull/19810/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvcmVwb3J0cy9ub3RpZmljYXRpb25zL3NsYWNrLnB5) | `34.72% <0.00%> (-52.95%)` | :arrow_down: |
   | [superset/reports/commands/execute.py](https://codecov.io/gh/apache/superset/pull/19810/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvcmVwb3J0cy9jb21tYW5kcy9leGVjdXRlLnB5) | `23.84% <8.33%> (-67.67%)` | :arrow_down: |
   | [...frontend/src/views/CRUD/alert/AlertReportModal.tsx](https://codecov.io/gh/apache/superset/pull/19810/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvYWxlcnQvQWxlcnRSZXBvcnRNb2RhbC50c3g=) | `52.08% <100.00%> (ø)` | |
   | [superset/models/reports.py](https://codecov.io/gh/apache/superset/pull/19810/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL3JlcG9ydHMucHk=) | `95.19% <100.00%> (-4.81%)` | :arrow_down: |
   | [superset/reports/notifications/base.py](https://codecov.io/gh/apache/superset/pull/19810/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvcmVwb3J0cy9ub3RpZmljYXRpb25zL2Jhc2UucHk=) | `88.00% <100.00%> (-7.84%)` | :arrow_down: |
   | [superset/utils/dashboard\_import\_export.py](https://codecov.io/gh/apache/superset/pull/19810/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvZGFzaGJvYXJkX2ltcG9ydF9leHBvcnQucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [superset/key\_value/commands/upsert.py](https://codecov.io/gh/apache/superset/pull/19810/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL3Vwc2VydC5weQ==) | `0.00% <0.00%> (-89.59%)` | :arrow_down: |
   | [superset/key\_value/commands/update.py](https://codecov.io/gh/apache/superset/pull/19810/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `0.00% <0.00%> (-89.37%)` | :arrow_down: |
   | ... and [276 more](https://codecov.io/gh/apache/superset/pull/19810/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/19810?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/19810?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [3db4a1c...658760b](https://codecov.io/gh/apache/superset/pull/19810?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] EugeneTorap commented on pull request #19810: feat(reports): xlsx attachments

Posted by GitBox <gi...@apache.org>.
EugeneTorap commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1277127765

   @cemremengu Thanks. Can you format a file with `black` to fix CI: Python Misc / pre-commit (3.8)?
   
   ```bash
   pip install black
   black superset/tests/integration_tests/reports/commands_tests.py
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] EugeneTorap commented on pull request #19810: feat(reports): xlsx attachments

Posted by GitBox <gi...@apache.org>.
EugeneTorap commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1276119909

   Hi @cemremengu. Any updates?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] cemremengu commented on a diff in pull request #19810: feat(reports): xlsx attachments

Posted by GitBox <gi...@apache.org>.
cemremengu commented on code in PR #19810:
URL: https://github.com/apache/superset/pull/19810#discussion_r993649747


##########
tests/integration_tests/reports/commands_tests.py:
##########
@@ -339,7 +339,7 @@ def create_report_slack_chart_with_csv():
         report_schedule = create_report_notification(
             slack_channel="slack_channel",
             chart=chart,
-            report_format=ReportDataFormat.DATA,
+            report_format=ReportDataFormat.CSV,

Review Comment:
   Done.



##########
superset/reports/commands/execute.py:
##########
@@ -333,11 +334,25 @@ def _get_notification_content(self) -> NotificationContent:
                     error_text = "Unexpected missing screenshot"
             elif (
                 self._report_schedule.chart
-                and self._report_schedule.report_format == ReportDataFormat.DATA
+                and self._report_schedule.report_format == ReportDataFormat.CSV
+            ):
+                data = self._get_csv_data()
+                if not data:
+                    error_text = "Unexpected missing csv file"
+            elif (
+                self._report_schedule.chart
+                and self._report_schedule.report_format == ReportDataFormat.XLSX
             ):
                 csv_data = self._get_csv_data()
                 if not csv_data:
-                    error_text = "Unexpected missing csv file"
+                    error_text = "Unexpected missing csv data for xlsx"
+                else:
+                    df = pd.read_csv(BytesIO(csv_data))
+                    bio = BytesIO()
+                    # pylint: disable=abstract-class-instantiated
+                    with pd.ExcelWriter(bio, engine="openpyxl") as writer:
+                        df.to_excel(writer, index=False)
+                    data = bio.getvalue()

Review Comment:
   Done.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] EugeneTorap commented on pull request #19810: feat(reports): xlsx attachments

Posted by GitBox <gi...@apache.org>.
EugeneTorap commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1277146601

   @cemremengu Can you also add Excel support in Download on explore page for consistency?
   
   ![Screenshot 2022-10-13 at 10 17 45](https://user-images.githubusercontent.com/29536522/195529237-43f69e73-7b58-4ea9-9c97-3240bfa9de32.png)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] smartsense-as commented on pull request #19810: feat(reports): xlsx attachments

Posted by GitBox <gi...@apache.org>.
smartsense-as commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1164381272

   Any update on this?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] github-actions[bot] commented on pull request #19810: feat(reports): xlsx attachments

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1397232245

   @rusackas Ephemeral environment spinning up at http://34.209.245.171:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] rusackas commented on pull request #19810: feat(reports): xlsx attachments

Posted by GitBox <gi...@apache.org>.
rusackas commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1347554852

   /testenv up


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] eschutho commented on a diff in pull request #19810: feat(reports): xlsx attachments

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19810:
URL: https://github.com/apache/superset/pull/19810#discussion_r975704282


##########
tests/integration_tests/reports/commands_tests.py:
##########
@@ -339,7 +339,7 @@ def create_report_slack_chart_with_csv():
         report_schedule = create_report_notification(
             slack_channel="slack_channel",
             chart=chart,
-            report_format=ReportDataFormat.DATA,
+            report_format=ReportDataFormat.CSV,

Review Comment:
   @cemremengu Thanks for the pr! Can you also include some tests like these that also use `XLSX` as the data format?



##########
superset/reports/commands/execute.py:
##########
@@ -333,11 +334,25 @@ def _get_notification_content(self) -> NotificationContent:
                     error_text = "Unexpected missing screenshot"
             elif (
                 self._report_schedule.chart
-                and self._report_schedule.report_format == ReportDataFormat.DATA
+                and self._report_schedule.report_format == ReportDataFormat.CSV
+            ):
+                data = self._get_csv_data()
+                if not data:
+                    error_text = "Unexpected missing csv file"
+            elif (
+                self._report_schedule.chart
+                and self._report_schedule.report_format == ReportDataFormat.XLSX
             ):
                 csv_data = self._get_csv_data()
                 if not csv_data:
-                    error_text = "Unexpected missing csv file"
+                    error_text = "Unexpected missing csv data for xlsx"
+                else:
+                    df = pd.read_csv(BytesIO(csv_data))
+                    bio = BytesIO()
+                    # pylint: disable=abstract-class-instantiated
+                    with pd.ExcelWriter(bio, engine="openpyxl") as writer:
+                        df.to_excel(writer, index=False)
+                    data = bio.getvalue()

Review Comment:
   I would recommend moving most of the csv and excel creation logic here to a new command called something like `_get_excel_data` so that it can be reusable, too. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] github-actions[bot] commented on pull request #19810: feat(reports): xlsx attachments

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1347556634

   @rusackas Ephemeral environment spinning up at http://35.86.251.160:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] cemremengu commented on a diff in pull request #19810: feat(reports): xlsx attachments

Posted by "cemremengu (via GitHub)" <gi...@apache.org>.
cemremengu commented on code in PR #19810:
URL: https://github.com/apache/superset/pull/19810#discussion_r1089736889


##########
superset/reports/commands/execute.py:
##########
@@ -330,13 +332,25 @@ def _get_log_data(self) -> HeaderDataType:
         }
         return log_data
 
+    def _get_xlsx_data(self) -> bytes:
+        csv_data = self._get_csv_data()
+
+        df = pd.read_csv(BytesIO(csv_data))

Review Comment:
   I am a bit confused about this part. We are getting df in order to convert it to excel. 
   
   I mean if we already have df we can just convert to excel so what would the common method achieve? Could you provide more details?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] rusackas commented on pull request #19810: feat(reports): xlsx attachments

Posted by GitBox <gi...@apache.org>.
rusackas commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1397230909

   @cemremengu it looks like there was a PR by @lyndsiWilliams where some translations are moved into constants in one of the files. Would you mind rebasing off `master` to conform to this change? Sorry we didn't get this one merged first ;) 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] rusackas commented on pull request #19810: feat(reports): xlsx attachments

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1587815886

   Easiest way to get CI logs again is to close/reopen it, or apply new commits (like fixing the merge conflicts would do)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] cemremengu commented on pull request #19810: feat(reports): xlsx attachments

Posted by "cemremengu (via GitHub)" <gi...@apache.org>.
cemremengu commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1592830575

   @rusackas thanks for pointing these out! Cypress tests are failing but those seem to be caused by something else?
   
   I decided to revert back to the old `_get_csv_data` way to benefit from the post processing. The only problem now seems that the excel contains the index column even though I export it with `Index=False`. Should just force drop the first column? Any ideas about that?
   
   ![image](https://github.com/apache/superset/assets/1297759/156b24b2-2b7f-4444-a658-ee21334ab975)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] kakoni commented on a diff in pull request #19810: feat(reports): xlsx attachments

Posted by "kakoni (via GitHub)" <gi...@apache.org>.
kakoni commented on code in PR #19810:
URL: https://github.com/apache/superset/pull/19810#discussion_r1101449494


##########
superset/reports/commands/execute.py:
##########
@@ -352,11 +366,18 @@ def _get_notification_content(self) -> NotificationContent:
                     error_text = "Unexpected missing screenshot"
             elif (
                 self._report_schedule.chart
-                and self._report_schedule.report_format == ReportDataFormat.DATA
+                and self._report_schedule.report_format == ReportDataFormat.CSV
             ):
-                csv_data = self._get_csv_data()
-                if not csv_data:
+                data = self._get_csv_data()

Review Comment:
   @EugeneTorap If you go this route (use `_get_data` instead of `_get_csv_data` then you lose the postprocessing that takes place in https://github.com/apache/superset/blob/ddd8d17aa4785918afc5395312678d206a2f100a/superset/charts/post_processing.py#L369-L373



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] rusackas commented on pull request #19810: feat(reports): xlsx attachments

Posted by GitBox <gi...@apache.org>.
rusackas commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1397225216

   /testenv up FEATURE_ALERT_REPORTS=true


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] EugeneTorap commented on pull request #19810: feat(reports): xlsx attachments

Posted by GitBox <gi...@apache.org>.
EugeneTorap commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1339140882

   @cemremengu Thanks. Can you format a file with black to fix CI: Python Misc / pre-commit (3.8)?
   ```
   pip install black
   black superset/tests/integration_tests/reports/commands_tests.py
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] EugeneTorap commented on pull request #19810: feat(reports): xlsx attachments

Posted by GitBox <gi...@apache.org>.
EugeneTorap commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1339166521

   I think yes, need to replace all UI label from `XLSX` to `Excel` for name consistency.
   @villebro @zhaoyongjie @michael-s-molina  What do you think?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] cemremengu commented on pull request #19810: feat(reports): xlsx attachments

Posted by GitBox <gi...@apache.org>.
cemremengu commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1339162528

   @EugeneTorap you mean replacing all occurrences of `XLSX` like  `EXPORT_TO_EXCEL` right ?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] cemremengu commented on pull request #19810: feat(reports): xlsx attachments

Posted by GitBox <gi...@apache.org>.
cemremengu commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1276408036

   @EugeneTorap thanks for the heads up! Just pushed the updates, my testing environment is limited now so apologies for any inconvenience


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [superset] EugeneTorap commented on pull request #19810: feat(reports): xlsx attachments

Posted by "EugeneTorap (via GitHub)" <gi...@apache.org>.
EugeneTorap commented on PR #19810:
URL: https://github.com/apache/superset/pull/19810#issuecomment-1408931364

   @geido @dpgaspar Can you review it? Maybe you have a good suggestion to improve the code?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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