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/10/20 03:28:53 UTC

[GitHub] [superset] Usiel opened a new pull request, #21879: feat(explore): add config for default time filter

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

   ### SUMMARY
   <!--- Describe the change below, including rationale and design decisions -->
   
   First change to complete https://github.com/apache/superset/issues/13125: We add a new configuration `DEFAULT_TIME_FILTER` to allow admins to set a global default time filter for any new explore charts. By default this is set to `None`, which replicates the previous behavior (`"No filter"`). To get a default filter of -1w the configuration can be set to `'Last week'` or any other value understood by Superset.
   
   There is a minor change in behavior `UPDATE_FORM_DATA_BY_DATASOURCE` (when switching dataset in explore), the time filter is not reset anymore. This is similar to the behavior of other globally configured filters such as the row limit for explore, which also do not reset.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <!--- Skip this if not applicable -->
   New explore chart with `DEFAULT_TIME_FILTER = None`:
   <img src="https://user-images.githubusercontent.com/1842905/196846013-317773cd-21e7-4ee3-889c-a16803874426.png" width="200">
   New explore chart with `DEFAULT_TIME_FILTER = "Last week"`:
   <img src="https://user-images.githubusercontent.com/1842905/196845999-d4838206-18d8-4ba3-bbdc-a9d93bed3957.png" width="200">
   
   ### TESTING INSTRUCTIONS
   
   #### Test default behavior
   1. Go to datasets and click on users (example dataset)
   2. Add `updated` (time) as a dimension (helps to verify)
   
   **Expected**: No time filter is applied when running the chart
   
   #### Test with new configuration
   1. Set `DEFAULT_TIME_FILTER = "2020-06-01T00:00:00 : now"` in `config.py`
   2. Go to datasets and click on users (example dataset)
   4. Add `updated` (time) as dimension (helps to verify)
   
   **Expected**: Default time filter is applied
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [x] Has associated issue: https://github.com/apache/superset/issues/13125
   - [ ] Required feature flags:
   - [ ] 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] Usiel commented on a diff in pull request #21879: feat(explore): add config for default time filter

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


##########
superset/config.py:
##########
@@ -153,6 +153,9 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]:
 SAMPLES_ROW_LIMIT = 1000
 # max rows retrieved by filter select auto complete
 FILTER_SELECT_ROW_LIMIT = 10000
+# default time filter in explore
+# values may be "Last day", "Last week", "<ISO date> : now", etc.
+DEFAULT_TIME_FILTER = None

Review Comment:
   Fixed πŸ‘ 



-- 
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] Utsavjain4561 commented on pull request #21879: feat(explore): add config for default time filter

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

   Hi, we are running superset 1.5.1rc1 right now and i guess in that there is no such support of `DEFAULT_TIME_FILTER` but i can see there a prop of `DEFAULT_TIME_RANGE` so is it possible if we hardcode its value to `Last week` and will it work for existing and new charts to have a default time range of `Last week` ?


-- 
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] zhaoyongjie merged pull request #21879: feat(explore): add config for default time filter

Posted by GitBox <gi...@apache.org>.
zhaoyongjie merged PR #21879:
URL: https://github.com/apache/superset/pull/21879


-- 
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 #21879: feat(explore): add config for default time filter

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

   # [Codecov](https://codecov.io/gh/apache/superset/pull/21879?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 [#21879](https://codecov.io/gh/apache/superset/pull/21879?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f1a73af) into [master](https://codecov.io/gh/apache/superset/commit/28c7636c591f39547f1201ff02da822e4a1bdf88?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (28c7636) will **decrease** coverage by `11.36%`.
   > The diff coverage is `66.66%`.
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #21879       +/-   ##
   ===========================================
   - Coverage   66.90%   55.54%   -11.37%     
   ===========================================
     Files        1806     1806               
     Lines       69130    69131        +1     
     Branches     7391     7391               
   ===========================================
   - Hits        46249    38396     -7853     
   - Misses      20970    28824     +7854     
     Partials     1911     1911               
   ```
   
   | Flag | Coverage Ξ” | |
   |---|---|---|
   | hive | `52.92% <100.00%> (+<0.01%)` | :arrow_up: |
   | mysql | `?` | |
   | postgres | `?` | |
   | presto | `52.82% <100.00%> (+<0.01%)` | :arrow_up: |
   | python | `57.94% <100.00%> (-23.54%)` | :arrow_down: |
   | sqlite | `?` | |
   | unit | `51.06% <100.00%> (+<0.01%)` | :arrow_up: |
   
   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/21879?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Ξ” | |
   |---|---|---|
   | [...d/packages/superset-ui-core/src/query/constants.ts](https://codecov.io/gh/apache/superset/pull/21879/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-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvcXVlcnkvY29uc3RhbnRzLnRz) | `100.00% <ΓΈ> (ΓΈ)` | |
   | [...nts/controls/DateFilterControl/DateFilterLabel.tsx](https://codecov.io/gh/apache/superset/pull/21879/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EYXRlRmlsdGVyQ29udHJvbC9EYXRlRmlsdGVyTGFiZWwudHN4) | `35.71% <ΓΈ> (ΓΈ)` | |
   | [superset-frontend/src/explore/fixtures.tsx](https://codecov.io/gh/apache/superset/pull/21879/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvZml4dHVyZXMudHN4) | `75.00% <ΓΈ> (ΓΈ)` | |
   | [...et-frontend/src/explore/reducers/exploreReducer.js](https://codecov.io/gh/apache/superset/pull/21879/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvcmVkdWNlcnMvZXhwbG9yZVJlZHVjZXIuanM=) | `38.09% <ΓΈ> (ΓΈ)` | |
   | [superset/views/base.py](https://codecov.io/gh/apache/superset/pull/21879/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-c3VwZXJzZXQvdmlld3MvYmFzZS5weQ==) | `60.56% <ΓΈ> (-15.15%)` | :arrow_down: |
   | [...set-frontend/src/explore/actions/hydrateExplore.ts](https://codecov.io/gh/apache/superset/pull/21879/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvYWN0aW9ucy9oeWRyYXRlRXhwbG9yZS50cw==) | `42.10% <50.00%> (ΓΈ)` | |
   | [superset/config.py](https://codecov.io/gh/apache/superset/pull/21879/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-c3VwZXJzZXQvY29uZmlnLnB5) | `91.02% <100.00%> (-0.62%)` | :arrow_down: |
   | [superset/utils/dashboard\_import\_export.py](https://codecov.io/gh/apache/superset/pull/21879/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/tags/core.py](https://codecov.io/gh/apache/superset/pull/21879/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-c3VwZXJzZXQvdGFncy9jb3JlLnB5) | `4.54% <0.00%> (-95.46%)` | :arrow_down: |
   | [superset/key\_value/commands/update.py](https://codecov.io/gh/apache/superset/pull/21879/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%> (-90.91%)` | :arrow_down: |
   | ... and [295 more](https://codecov.io/gh/apache/superset/pull/21879/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) | |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?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] zhaoyongjie commented on a diff in pull request #21879: feat(explore): add config for default time filter

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


##########
superset/config.py:
##########
@@ -153,6 +153,9 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]:
 SAMPLES_ROW_LIMIT = 1000
 # max rows retrieved by filter select auto complete
 FILTER_SELECT_ROW_LIMIT = 10000
+# default time filter in explore
+# values may be "Last day", "Last week", "<ISO date> : now", etc.
+DEFAULT_TIME_FILTER = None

Review Comment:
   one nite, we should keep this config as before, the `None` and the `No filter` will generate different time expressions. the `None` will generate ` : today`(col <= today()), and `No filter` will generate ` : `(empty filter).



-- 
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] Usiel commented on pull request #21879: feat(explore): add config for default time filter

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

   Apologies for the forced pushes, was rebasing and missed something during my merge. I should be done now :)


-- 
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] zhaoyongjie commented on pull request #21879: feat(explore): add config for default time filter

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

   > Hi, we are running superset 1.5.1rc1 right now and i guess in that there is no such support of `DEFAULT_TIME_FILTER` but i can see there a prop of `DEFAULT_TIME_RANGE` so is it possible if we hardcode its value to `Last week` and will it work for existing and new charts to have a default time range of `Last week` ?
   
   Yes, it should be same as `DEFAULT_TIME_FILTER` in config.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