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 2021/02/08 22:16:14 UTC

[GitHub] [superset] ktmud opened a new pull request #13015: fix: time filter migration optimization

ktmud opened a new pull request #13015:
URL: https://github.com/apache/superset/pull/13015


   ### SUMMARY
   
   Optimizes free-text time range filter db migration added by #12505 
   
   1. Update the validation Exception so it can be exposed to the frontend.
   2. Update the migration script so it can
        a) skip charts already with a valid filter, e.g. (`90 days before: 100 days later`)
        b) keep cases like `90 days ago : 30 days ago` untouched.  Previously it will incorrectly change it to `90 days ago : 30 days later`.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   #### Before
   
   When opening a chart with "100 years" (i.e. periods without "ago" or "before/after"), it shows "Fatal error":
   
   <img width="848" alt="request-is-incorret" src="https://user-images.githubusercontent.com/335541/107283248-fbb69500-6a10-11eb-806e-90e337ee2461.png">
   
   #### After
   
   The error is more actionable:
   
   <img width="848" alt="request-is-incorret" src="https://user-images.githubusercontent.com/335541/107283231-f35e5a00-6a10-11eb-9779-207b4adc251e.png">
   
   (Still need some optimization here in order to render the backend validation error more properly but let's do it in another PR for other fields as well. Ideally the backend validation errors should come back to each control based on field name.)
   
   ### TEST PLAN
   
   #12552 is for fixing legacy charts. You will not be able to add bad data anymore because of the backend validation. You'd need to either find a legacy bad chart, or manually add one. A quick way to do it is this:
   
   1. Connect to your Superset db using any database manager
   
   For example I use `psql`:
   
   ```
   psql "postgresql://superset:superset@127.0.0.1:5432/superset_test"
   ```
   
   2. Run following SQL command:
   
   ```sql
   INSERT INTO
     slices (
       created_on,
       changed_on,
       slice_name,
       datasource_type,
       datasource_name,
       viz_type,
       params,
       perm,
       datasource_id
     )
   VALUES
     (
       '2021-01-25T13:21:40.763807',
       '2021-01-25T13:21:40.763807',
       'Test time period',
       'table',
       null,
       'table',
       '{ "compare_lag": "10", "compare_suffix": "o10Y", "country_fieldtype": "cca3", "entity": "country_code", "granularity_sqla": "year", "groupby": [ "country_name" ], "limit": "25", "markup_type": "markdown", "metrics": [ "sum__SP_POP_TOTL" ], "row_limit": 50000, "show_bubbles": true, "time_range": "100 years : 30 years ago", "time_range_endpoints": [ "inclusive", "exclusive" ], "viz_type": "table" }',
       '[examples].[wb_health_population](id:2)',
       2
     );
   ```
   
   4. Find the ID of the chart you just inserted:
   
   ```sql
   SELECT *
   FROM slices
   WHERE slice_name like 'Test time period'
   ```
   
   3. Go to `/superset/slice/{slice_id}`
   
   
   Before migration, you should see the "Unexpected error" when you open the chart.
   
   After migration, you should see the chart render correctly. 
   
   Note: if you have already upgraded superset db, you need to downgrade it first:
   
   ```
   superset db downgrade c878781977c6 
   superset db upgrade
   ```
   
   When in current master, after migration, you will also see the filter being updated to "100 years ago : 30 years later":
   
   <img src="https://user-images.githubusercontent.com/335541/107286526-8e593300-6a15-11eb-820c-b14113e1c3e7.png" width="300">
   
   With this PR, after migration, you should see the filter being updated to "100 years ago : 30 years ago":
   
   <img src="https://user-images.githubusercontent.com/335541/107287959-91552300-6a17-11eb-8bea-9da72bbe92b3.png" width="300">
   
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [x] Changes UI
   - [x] Requires DB Migration.
   - [x] Confirm DB Migration upgrade and downgrade tested.
   - [ ] 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.

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] ktmud commented on a change in pull request #13015: fix: time filter db migration optimization

Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #13015:
URL: https://github.com/apache/superset/pull/13015#discussion_r572419372



##########
File path: superset/charts/commands/exceptions.py
##########
@@ -28,13 +28,41 @@
 )
 
 
+class TimeRangeUnclearError(ValidationError):
+    """
+    Time range is in valid error.
+    """
+
+    def __init__(self, human_readable: str) -> None:
+        super().__init__(
+            _(
+                "Time string is unclear."
+                " Please specify [%(human_readable)s ago]"
+                " or [%(human_readable)s later].",
+                human_readable=human_readable,
+            ),
+            field_name="time_range",
+        )
+
+
+class TimeRangeParseFailError(ValidationError):
+    def __init__(self, human_readable: str) -> None:
+        super().__init__(
+            _(
+                "Cannot parse time string [%(human_readable)s]",
+                human_readable=human_readable,
+            ),
+            field_name="time_range",
+        )
+
+
 class DatabaseNotFoundValidationError(ValidationError):
     """
     Marshmallow validation error for database does not exist
     """
 
     def __init__(self) -> None:
-        super().__init__(_("Database does not exist"), field_names=["database"])
+        super().__init__(_("Database does not exist"), field_name="database")

Review comment:
       Bycatch: should use [`field_name`](https://marshmallow.readthedocs.io/en/stable/marshmallow.exceptions.html#marshmallow.ValidationError) instead of `field_names`.




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

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



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

[GitHub] [superset] codecov-io commented on pull request #13015: fix: time filter db migration optimization

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #13015:
URL: https://github.com/apache/superset/pull/13015#issuecomment-775595189


   # [Codecov](https://codecov.io/gh/apache/superset/pull/13015?src=pr&el=h1) Report
   > Merging [#13015](https://codecov.io/gh/apache/superset/pull/13015?src=pr&el=desc) (0907b6f) into [master](https://codecov.io/gh/apache/superset/commit/50fa10054fd5d2c28553c4ffd754e12bdedef5da?el=desc) (50fa100) will **increase** coverage by `14.32%`.
   > The diff coverage is `60.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/13015/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/superset/pull/13015?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #13015       +/-   ##
   ===========================================
   + Coverage   52.64%   66.97%   +14.32%     
   ===========================================
     Files         480      489        +9     
     Lines       17300    28764    +11464     
     Branches     4481        0     -4481     
   ===========================================
   + Hits         9108    19264    +10156     
   - Misses       8192     9500     +1308     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `?` | |
   | python | `66.97% <60.00%> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/13015?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...s/260bf0649a77\_migrate\_x\_dateunit\_in\_time\_range.py](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbWlncmF0aW9ucy92ZXJzaW9ucy8yNjBiZjA2NDlhNzdfbWlncmF0ZV94X2RhdGV1bml0X2luX3RpbWVfcmFuZ2UucHk=) | `0.00% <0.00%> (ø)` | |
   | [superset/charts/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2hhcnRzL2NvbW1hbmRzL2V4Y2VwdGlvbnMucHk=) | `92.85% <77.77%> (ø)` | |
   | [superset/utils/date\_parser.py](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGF0ZV9wYXJzZXIucHk=) | `96.87% <100.00%> (ø)` | |
   | [...et-frontend/src/dashboard/actions/sliceEntities.js](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9hY3Rpb25zL3NsaWNlRW50aXRpZXMuanM=) | | |
   | [...rontend/src/dashboard/util/shouldWrapChildInRow.js](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL3Nob3VsZFdyYXBDaGlsZEluUm93Lmpz) | | |
   | [...d/src/dashboard/util/updateComponentParentsList.js](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL3VwZGF0ZUNvbXBvbmVudFBhcmVudHNMaXN0Lmpz) | | |
   | [superset-frontend/src/utils/parseCookie.ts](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3V0aWxzL3BhcnNlQ29va2llLnRz) | | |
   | [.../components/ErrorMessage/ParameterErrorMessage.tsx](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRXJyb3JNZXNzYWdlL1BhcmFtZXRlckVycm9yTWVzc2FnZS50c3g=) | | |
   | [...nd/src/dashboard/util/getDetailedComponentWidth.js](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldERldGFpbGVkQ29tcG9uZW50V2lkdGguanM=) | | |
   | [superset-frontend/src/components/ListView/types.ts](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXcvdHlwZXMudHM=) | | |
   | ... and [944 more](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/13015?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/13015?src=pr&el=footer). Last update [eb5dc38...0907b6f](https://codecov.io/gh/apache/superset/pull/13015?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


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

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



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

[GitHub] [superset] ktmud merged pull request #13015: fix: time filter db migration optimization

Posted by GitBox <gi...@apache.org>.
ktmud merged pull request #13015:
URL: https://github.com/apache/superset/pull/13015


   


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

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



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

[GitHub] [superset] codecov-io edited a comment on pull request #13015: fix: time filter db migration optimization

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #13015:
URL: https://github.com/apache/superset/pull/13015#issuecomment-775595189


   # [Codecov](https://codecov.io/gh/apache/superset/pull/13015?src=pr&el=h1) Report
   > Merging [#13015](https://codecov.io/gh/apache/superset/pull/13015?src=pr&el=desc) (0be83f0) into [master](https://codecov.io/gh/apache/superset/commit/50fa10054fd5d2c28553c4ffd754e12bdedef5da?el=desc) (50fa100) will **increase** coverage by `14.32%`.
   > The diff coverage is `60.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/13015/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/superset/pull/13015?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #13015       +/-   ##
   ===========================================
   + Coverage   52.64%   66.97%   +14.32%     
   ===========================================
     Files         480      489        +9     
     Lines       17300    28766    +11466     
     Branches     4481        0     -4481     
   ===========================================
   + Hits         9108    19266    +10158     
   - Misses       8192     9500     +1308     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `?` | |
   | python | `66.97% <60.00%> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/13015?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...s/260bf0649a77\_migrate\_x\_dateunit\_in\_time\_range.py](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbWlncmF0aW9ucy92ZXJzaW9ucy8yNjBiZjA2NDlhNzdfbWlncmF0ZV94X2RhdGV1bml0X2luX3RpbWVfcmFuZ2UucHk=) | `0.00% <0.00%> (ø)` | |
   | [superset/charts/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2hhcnRzL2NvbW1hbmRzL2V4Y2VwdGlvbnMucHk=) | `92.85% <77.77%> (ø)` | |
   | [superset/utils/date\_parser.py](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGF0ZV9wYXJzZXIucHk=) | `96.87% <100.00%> (ø)` | |
   | [superset-frontend/src/SqlLab/constants.ts](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb25zdGFudHMudHM=) | | |
   | [...rset-frontend/src/components/ImportModal/index.tsx](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvSW1wb3J0TW9kYWwvaW5kZXgudHN4) | | |
   | [...end/src/dashboard/util/getKeyForFilterScopeTree.js](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEtleUZvckZpbHRlclNjb3BlVHJlZS5qcw==) | | |
   | [.../controls/MetricControl/FilterDefinitionOption.jsx](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0ZpbHRlckRlZmluaXRpb25PcHRpb24uanN4) | | |
   | [...rc/explore/components/controls/ColorMapControl.jsx](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Db2xvck1hcENvbnRyb2wuanN4) | | |
   | [...tend/src/dashboard/containers/DashboardBuilder.jsx](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0Rhc2hib2FyZEJ1aWxkZXIuanN4) | | |
   | [superset-frontend/src/chart/chartReducer.js](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NoYXJ0L2NoYXJ0UmVkdWNlci5qcw==) | | |
   | ... and [944 more](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/13015?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/13015?src=pr&el=footer). Last update [eb5dc38...0be83f0](https://codecov.io/gh/apache/superset/pull/13015?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


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

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



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

[GitHub] [superset] codecov-io edited a comment on pull request #13015: fix: time filter db migration optimization

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #13015:
URL: https://github.com/apache/superset/pull/13015#issuecomment-775595189


   # [Codecov](https://codecov.io/gh/apache/superset/pull/13015?src=pr&el=h1) Report
   > Merging [#13015](https://codecov.io/gh/apache/superset/pull/13015?src=pr&el=desc) (0be83f0) into [master](https://codecov.io/gh/apache/superset/commit/50fa10054fd5d2c28553c4ffd754e12bdedef5da?el=desc) (50fa100) will **increase** coverage by `14.32%`.
   > The diff coverage is `60.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/13015/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/superset/pull/13015?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #13015       +/-   ##
   ===========================================
   + Coverage   52.64%   66.97%   +14.32%     
   ===========================================
     Files         480      489        +9     
     Lines       17300    28764    +11464     
     Branches     4481        0     -4481     
   ===========================================
   + Hits         9108    19264    +10156     
   - Misses       8192     9500     +1308     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `?` | |
   | python | `66.97% <60.00%> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/13015?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...s/260bf0649a77\_migrate\_x\_dateunit\_in\_time\_range.py](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbWlncmF0aW9ucy92ZXJzaW9ucy8yNjBiZjA2NDlhNzdfbWlncmF0ZV94X2RhdGV1bml0X2luX3RpbWVfcmFuZ2UucHk=) | `0.00% <0.00%> (ø)` | |
   | [superset/charts/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2hhcnRzL2NvbW1hbmRzL2V4Y2VwdGlvbnMucHk=) | `92.85% <77.77%> (ø)` | |
   | [superset/utils/date\_parser.py](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGF0ZV9wYXJzZXIucHk=) | `96.87% <100.00%> (ø)` | |
   | [superset-frontend/src/SqlLab/constants.ts](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb25zdGFudHMudHM=) | | |
   | [...rset-frontend/src/components/ImportModal/index.tsx](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvSW1wb3J0TW9kYWwvaW5kZXgudHN4) | | |
   | [...end/src/dashboard/util/getKeyForFilterScopeTree.js](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEtleUZvckZpbHRlclNjb3BlVHJlZS5qcw==) | | |
   | [.../controls/MetricControl/FilterDefinitionOption.jsx](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0ZpbHRlckRlZmluaXRpb25PcHRpb24uanN4) | | |
   | [...rc/explore/components/controls/ColorMapControl.jsx](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Db2xvck1hcENvbnRyb2wuanN4) | | |
   | [...tend/src/dashboard/containers/DashboardBuilder.jsx](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0Rhc2hib2FyZEJ1aWxkZXIuanN4) | | |
   | [superset-frontend/src/chart/chartReducer.js](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NoYXJ0L2NoYXJ0UmVkdWNlci5qcw==) | | |
   | ... and [944 more](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/13015?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/13015?src=pr&el=footer). Last update [eb5dc38...0be83f0](https://codecov.io/gh/apache/superset/pull/13015?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


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

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



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

[GitHub] [superset] zhaoyongjie commented on a change in pull request #13015: fix: time filter db migration optimization

Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on a change in pull request #13015:
URL: https://github.com/apache/superset/pull/13015#discussion_r572538754



##########
File path: tests/utils/date_parser_tests.py
##########
@@ -291,3 +295,6 @@ def test_DateRangeMigration(self):
 
         field = "   8 days   "
         self.assertRegex(field, DateRangeMigration.x_dateunit)
+
+        field = "last week"
+        self.assertNotRegex(field, DateRangeMigration.x_dateunit)

Review comment:
       One nit, add a test case :
   ```
           field = "1 years ago"
           self.assertNotRegex(field, DateRangeMigration.x_dateunit)
   ```




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

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



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

[GitHub] [superset] ktmud commented on a change in pull request #13015: fix: time filter db migration optimization

Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #13015:
URL: https://github.com/apache/superset/pull/13015#discussion_r572418495



##########
File path: superset/charts/commands/exceptions.py
##########
@@ -14,7 +14,7 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-from flask_babel import lazy_gettext as _
+from flask_babel import _

Review comment:
       Bycatch, lazy text cannot be properly formatted by Marshmallow.




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

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



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

[GitHub] [superset] zhaoyongjie commented on a change in pull request #13015: fix: time filter db migration optimization

Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on a change in pull request #13015:
URL: https://github.com/apache/superset/pull/13015#discussion_r572545245



##########
File path: tests/utils/date_parser_tests.py
##########
@@ -291,3 +295,6 @@ def test_DateRangeMigration(self):
 
         field = "   8 days   "
         self.assertRegex(field, DateRangeMigration.x_dateunit)
+
+        field = "last week"
+        self.assertNotRegex(field, DateRangeMigration.x_dateunit)

Review comment:
       One nit, add a test case :
   ```suggestion
   
           field = "last week"
           self.assertNotRegex(field, DateRangeMigration.x_dateunit)
           
           field = "1 years ago"
           self.assertNotRegex(field, DateRangeMigration.x_dateunit)
   ```




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

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



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

[GitHub] [superset] zhaoyongjie commented on a change in pull request #13015: fix: time filter db migration optimization

Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on a change in pull request #13015:
URL: https://github.com/apache/superset/pull/13015#discussion_r572545245



##########
File path: tests/utils/date_parser_tests.py
##########
@@ -291,3 +295,6 @@ def test_DateRangeMigration(self):
 
         field = "   8 days   "
         self.assertRegex(field, DateRangeMigration.x_dateunit)
+
+        field = "last week"
+        self.assertNotRegex(field, DateRangeMigration.x_dateunit)

Review comment:
       ```suggestion
   
           field = "last week"
           self.assertNotRegex(field, DateRangeMigration.x_dateunit)
           
           field = "1 years ago"
           self.assertNotRegex(field, DateRangeMigration.x_dateunit)
   ```




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

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



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

[GitHub] [superset] zhaoyongjie closed pull request #13015: fix: time filter db migration optimization

Posted by GitBox <gi...@apache.org>.
zhaoyongjie closed pull request #13015:
URL: https://github.com/apache/superset/pull/13015


   


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

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



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

[GitHub] [superset] zhaoyongjie commented on a change in pull request #13015: fix: time filter db migration optimization

Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on a change in pull request #13015:
URL: https://github.com/apache/superset/pull/13015#discussion_r572538754



##########
File path: tests/utils/date_parser_tests.py
##########
@@ -291,3 +295,6 @@ def test_DateRangeMigration(self):
 
         field = "   8 days   "
         self.assertRegex(field, DateRangeMigration.x_dateunit)
+
+        field = "last week"
+        self.assertNotRegex(field, DateRangeMigration.x_dateunit)

Review comment:
       One nit, add a test case :
   ```
           field = "1 years ago"
           self.assertNotRegex(field, DateRangeMigration.x_dateunit)
   ```




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

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



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

[GitHub] [superset] codecov-io edited a comment on pull request #13015: fix: time filter db migration optimization

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #13015:
URL: https://github.com/apache/superset/pull/13015#issuecomment-775595189


   # [Codecov](https://codecov.io/gh/apache/superset/pull/13015?src=pr&el=h1) Report
   > Merging [#13015](https://codecov.io/gh/apache/superset/pull/13015?src=pr&el=desc) (0907b6f) into [master](https://codecov.io/gh/apache/superset/commit/50fa10054fd5d2c28553c4ffd754e12bdedef5da?el=desc) (50fa100) will **increase** coverage by `14.32%`.
   > The diff coverage is `60.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/13015/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/superset/pull/13015?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #13015       +/-   ##
   ===========================================
   + Coverage   52.64%   66.97%   +14.32%     
   ===========================================
     Files         480      489        +9     
     Lines       17300    28766    +11466     
     Branches     4481        0     -4481     
   ===========================================
   + Hits         9108    19266    +10158     
   - Misses       8192     9500     +1308     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `?` | |
   | python | `66.97% <60.00%> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/13015?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...s/260bf0649a77\_migrate\_x\_dateunit\_in\_time\_range.py](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbWlncmF0aW9ucy92ZXJzaW9ucy8yNjBiZjA2NDlhNzdfbWlncmF0ZV94X2RhdGV1bml0X2luX3RpbWVfcmFuZ2UucHk=) | `0.00% <0.00%> (ø)` | |
   | [superset/charts/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2hhcnRzL2NvbW1hbmRzL2V4Y2VwdGlvbnMucHk=) | `92.85% <77.77%> (ø)` | |
   | [superset/utils/date\_parser.py](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGF0ZV9wYXJzZXIucHk=) | `96.87% <100.00%> (ø)` | |
   | [...et-frontend/src/filters/components/Select/index.ts](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZpbHRlcnMvY29tcG9uZW50cy9TZWxlY3QvaW5kZXgudHM=) | | |
   | [.../src/explore/components/controls/BoundsControl.jsx](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Cb3VuZHNDb250cm9sLmpzeA==) | | |
   | [superset-frontend/src/components/ChartIcon.tsx](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvQ2hhcnRJY29uLnRzeA==) | | |
   | [superset-frontend/src/utils/hostNamesConfig.js](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3V0aWxzL2hvc3ROYW1lc0NvbmZpZy5qcw==) | | |
   | [...rontend/src/visualizations/FilterBox/FilterBox.jsx](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3Zpc3VhbGl6YXRpb25zL0ZpbHRlckJveC9GaWx0ZXJCb3guanN4) | | |
   | [...rset-frontend/src/components/AsyncEsmComponent.tsx](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvQXN5bmNFc21Db21wb25lbnQudHN4) | | |
   | [...nd/src/messageToasts/containers/ToastPresenter.jsx](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL21lc3NhZ2VUb2FzdHMvY29udGFpbmVycy9Ub2FzdFByZXNlbnRlci5qc3g=) | | |
   | ... and [944 more](https://codecov.io/gh/apache/superset/pull/13015/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/13015?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/13015?src=pr&el=footer). Last update [eb5dc38...0907b6f](https://codecov.io/gh/apache/superset/pull/13015?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


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

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



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