You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "craig-rueda (via GitHub)" <gi...@apache.org> on 2023/06/02 22:21:47 UTC

[GitHub] [superset] craig-rueda opened a new pull request, #24279: fix(migrations): Fixing cross filter migration

craig-rueda opened a new pull request, #24279:
URL: https://github.com/apache/superset/pull/24279

   ### SUMMARY
   Fixing bug in down migration for Cross Filtering


-- 
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] sadpandajoe commented on pull request #24279: fix(migrations): Fixing cross filter migration

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

   🏷️ preset:2023.21


-- 
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] jinghua-qa merged pull request #24279: fix(migrations): Fixing cross filter migration

Posted by "jinghua-qa (via GitHub)" <gi...@apache.org>.
jinghua-qa merged PR #24279:
URL: https://github.com/apache/superset/pull/24279


-- 
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] betodealmeida commented on a diff in pull request #24279: fix(migrations): Fixing cross filter migration

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


##########
superset/migrations/versions/2023-05-11_12-41_4ea966691069_cross_filter_global_scoping.py:
##########
@@ -91,20 +94,24 @@ def downgrade():
                 chart_id = config.get("id")
                 if chart_id is None:
                     continue
-                scope = config.get("crossFilters", {}).get("scope", {})
+                scope = config.get("crossFilters", {}).get("scope", "")
                 new_chart_configuration[chart_id] = copy.deepcopy(config)
-                if scope == "global":
+                if scope.lower() == "global":
                     new_chart_configuration[chart_id]["crossFilters"]["scope"] = {
                         "rootPath": ["ROOT_ID"],
                         "excluded": [chart_id],
                     }
 
             json_metadata["chart_configuration"] = new_chart_configuration
-            del json_metadata["global_chart_configuration"]
+
+            if "global_chart_configuration" in json_metadata:
+                del json_metadata["global_chart_configuration"]
+

Review Comment:
   `.pop()` fails if the key doesn't exist:
   
   ```python
   >>> {}.pop(1)
   Traceback (most recent call last):
     File "<stdin>", line 1, in <module>
   KeyError: 1
   ```



-- 
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 #24279: fix(migrations): Fixing cross filter migration

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #24279:
URL: https://github.com/apache/superset/pull/24279#issuecomment-1574389592

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/24279?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   > Merging [#24279](https://app.codecov.io/gh/apache/superset/pull/24279?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (745c220) into [master](https://app.codecov.io/gh/apache/superset/commit/1d9a761de5410fa1bd208bca4c78614779cf3064?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (1d9a761) will **decrease** coverage by `11.05%`.
   > The diff coverage is `n/a`.
   
   > :exclamation: Current head 745c220 differs from pull request most recent head 72d193c. Consider uploading reports for the commit 72d193c to get more accurate results
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #24279       +/-   ##
   ===========================================
   - Coverage   68.29%   57.24%   -11.05%     
   ===========================================
     Files        1957     1957               
     Lines       75624    75624               
     Branches     8223     8223               
   ===========================================
   - Hits        51649    43294     -8355     
   - Misses      21867    30222     +8355     
     Partials     2108     2108               
   ```
   
   | Flag | Coverage Ξ” | |
   |---|---|---|
   | hive | `53.39% <ΓΈ> (ΓΈ)` | |
   | postgres | `?` | |
   | presto | `53.32% <ΓΈ> (ΓΈ)` | |
   | python | `59.93% <ΓΈ> (-22.85%)` | :arrow_down: |
   | sqlite | `?` | |
   | unit | `53.44% <ΓΈ> (ΓΈ)` | |
   
   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=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   [see 306 files with indirect coverage changes](https://app.codecov.io/gh/apache/superset/pull/24279/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   
   :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=apache)
   


-- 
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] hughhhh commented on a diff in pull request #24279: fix(migrations): Fixing cross filter migration

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


##########
superset/migrations/versions/2023-05-11_12-41_4ea966691069_cross_filter_global_scoping.py:
##########
@@ -91,20 +94,24 @@ def downgrade():
                 chart_id = config.get("id")
                 if chart_id is None:
                     continue
-                scope = config.get("crossFilters", {}).get("scope", {})
+                scope = config.get("crossFilters", {}).get("scope", "")
                 new_chart_configuration[chart_id] = copy.deepcopy(config)
-                if scope == "global":
+                if scope.lower() == "global":
                     new_chart_configuration[chart_id]["crossFilters"]["scope"] = {
                         "rootPath": ["ROOT_ID"],
                         "excluded": [chart_id],
                     }
 
             json_metadata["chart_configuration"] = new_chart_configuration
-            del json_metadata["global_chart_configuration"]
+
+            if "global_chart_configuration" in json_metadata:
+                del json_metadata["global_chart_configuration"]
+

Review Comment:
   nit: 
   ```suggestion
               json_metadata.pop('global_chart_configuration')
   ```



-- 
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] hughhhh commented on a diff in pull request #24279: fix(migrations): Fixing cross filter migration

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


##########
superset/migrations/versions/2023-05-11_12-41_4ea966691069_cross_filter_global_scoping.py:
##########
@@ -91,20 +94,24 @@ def downgrade():
                 chart_id = config.get("id")
                 if chart_id is None:
                     continue
-                scope = config.get("crossFilters", {}).get("scope", {})
+                scope = config.get("crossFilters", {}).get("scope", "")
                 new_chart_configuration[chart_id] = copy.deepcopy(config)
-                if scope == "global":
+                if scope.lower() == "global":
                     new_chart_configuration[chart_id]["crossFilters"]["scope"] = {
                         "rootPath": ["ROOT_ID"],
                         "excluded": [chart_id],
                     }
 
             json_metadata["chart_configuration"] = new_chart_configuration
-            del json_metadata["global_chart_configuration"]
+
+            if "global_chart_configuration" in json_metadata:

Review Comment:
   nit: json_metadata.pop('global_chart_configuration')



-- 
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