You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "betodealmeida (via GitHub)" <gi...@apache.org> on 2023/09/06 04:29:41 UTC

[GitHub] [superset] betodealmeida opened a new pull request, #25196: feat: mechanism for re-saving charts

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

   <!---
   Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
   Example:
   fix(dashboard): load charts correctly
   -->
   
   ### SUMMARY
   <!--- Describe the change below, including rationale and design decisions -->
   
   There's currently a bug in Superset where (1) if a chart is saved with a `granularity_sqla`, and (2) the `GENERIC_CHART_AXIS` feature is enabled after the chart creation, then the chart will break. This happens for some charts. Stripping `granularity_sqla` is not an option, because some charts still need it even when `GENERIC_CHART_AXIS` is enabled.
   
   @Vitor-Avila found a workaround where saving the chart again will fix it. So in this PR I'm adding a new column to charts called `force_save`. When the column is true, the chart will save itself when loaded (in explore or a dashboard), and the column will be set to false. This way, charts can be forced to save by setting this column to true, which could be done conditionally (for certain viz types, eg).
   
   Note that while this is a hack, it's a pattern we've used before. For alerts and reports we had to introduce logic to re-save the chart in order to set `query_context`. See https://github.com/apache/superset/pull/15824, https://github.com/apache/superset/pull/15865, and https://github.com/apache/superset/pull/15846.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <!--- Skip this if not applicable -->
   
   ### TESTING INSTRUCTIONS
   <!--- Required! What steps can be taken to manually verify the changes? -->
   
   Still in progress.
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] 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
   - [ ] 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] villebro commented on pull request #25196: feat: mechanism for re-saving charts

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

   I think in the long run we should really consider implementing a simple Node service that can render query context on the fly from form data (we could place this logic fairly easily in the websocket server). This would remove the need for persisting query context, which we can't update without resaving the chart if the `buildQuery` logic changes.


-- 
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 pull request #25196: feat: mechanism for re-saving charts

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

   > @betodealmeida @michael-s-molina while there's definitely a lot of standardization that can be done, I think this will be very difficult to achieve. For example, the table chart triggers different queries if the pagination control is checked. Just looking at existing `buildQueryContext` hooks in the existing plugins shows that constructing chart data requests is non-trivial, and often very viz-specific. Therefore, moving this logic to the backend would be very difficult, as it would essentially require moving business logic into the backend. This was one of the main bottlenecks of `viz.py`, and one of the motivations for SIP-6: #5692. But if we can indeed figure out a backend architecture that can facilitate arbitrary viz plugin logic without custom `buildQueryContext` hooks, this would indeed be a great simplification
   
   I definitely don't want to go back to the times of `viz.py`. Having the logic live only in JS makes it much easier to build and distribute visualization plugins, even if it introduces complexity (like sending an HTML report of a pivot table, where a lot of the transformations happen in JS).
   
   But I think we can generalize the request. At the end of the day we need the following in order to get data (I might be forgetting something):
   
   - metrics (adhoc or already defined)
   - dimensions
   - filters (`WHERE` and `HAVING`)
   - limit, offset, ordering
   - post processing operations
   
   And then each viz type has its own controls for presentation. Also, each viz should have a standard method for data transformation, something like `export function processRows(rows: Row[]): Row[] {}`, so that it can be called from outside the plugin (for alerts and reports, eg).


-- 
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 pull request #25196: feat: mechanism for re-saving charts

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

   > I think this work is highly correlated with the new plugin architecture we were thinking about.
   
   Can you share more about this? Can we have a bigger discussion?


-- 
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 pull request #25196: feat: mechanism for re-saving charts

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

   > I think in the long run we should really consider implementing a simple Node service that can render query context on the fly from form data (we could place this logic fairly easily in the websocket server). This would remove the need for persisting query context, which we can't update without resaving the chart if the `buildQuery` logic changes.
   
   I don't know if it's possible, but what I'd like instead is a simpler contract between visualizations and the backend, so that this isn't even needed.


-- 
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 pull request #25196: feat: mechanism for re-saving charts

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

   Maybe when @michael-s-molina is in town we could do a committer meetup and discuss this and other similar topics? I'd love to talk about Flask blueprints and my vision for them. :)


-- 
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] michael-s-molina commented on pull request #25196: feat: mechanism for re-saving charts

Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on PR #25196:
URL: https://github.com/apache/superset/pull/25196#issuecomment-1708810600

   Thank you for the additional context @betodealmeida.
   
   > I'm still running tests to see which charts are affected and if this PR can fix them.
   
   Got it. Let me know the results.
   
   > I don't know if it's possible, but what I'd like instead is a simpler contract between visualizations and the backend, so that this isn't even needed.
   
   Me 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] zhaoyongjie commented on pull request #25196: feat: mechanism for re-saving charts

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

   @betodealmeida 
   This issue might introduce from [PR](https://github.com/apache/superset/pull/23021), basically, we don't need transformation in the backend, it should transform in the frontend.
   
   The other hand, I think we should try to avoid hacks too much in Superset, hacks have too much for 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] michael-s-molina commented on pull request #25196: feat: mechanism for re-saving charts

Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on PR #25196:
URL: https://github.com/apache/superset/pull/25196#issuecomment-1708453696

   Thank you for the PR @betodealmeida. We have been discussing this internally at Airbnb and it's clear that these problems happen because we are missing schema versioning and validation for these types of fields. We have many bugs that were reported because these schemas change every time we introduce a new feature, essentially creating a new version of it, but we don't correctly support/migrate old versions. The fact that many of these schema transformations are on the frontend is really problematic because we end up creating columns in the database to store those transformations so they can be used by Python code when processing API requests as you can see by https://github.com/apache/superset/pull/15824, https://github.com/apache/superset/pull/15865, and https://github.com/apache/superset/pull/15846. @john-bodley Here's the motivation for the `params` and `query_context` duplication that haunts us 😄 
   
   Another critical problem of that approach is that Superset assets can be consumed through APIs, without a frontend, and in that case, changes in feature flags don't reflect in the schemas returned by these APIs. We have 20 feature flags that will be removed in 4.0 (including `GENERIC_CHART_AXES`) which will ease the problem but we still need to discuss schema versioning for these types of fields.
   
   I understand the motivation for this PR but before we go down this road, is it possible to fix the issue using other means? Can you explain what breaks (video, stacktrace) and which chart types are affected?


-- 
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 closed pull request #25196: feat: mechanism for re-saving charts

Posted by "betodealmeida (via GitHub)" <gi...@apache.org>.
betodealmeida closed pull request #25196: feat: mechanism for re-saving charts
URL: https://github.com/apache/superset/pull/25196


-- 
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] michael-s-molina commented on pull request #25196: feat: mechanism for re-saving charts

Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on PR #25196:
URL: https://github.com/apache/superset/pull/25196#issuecomment-1708920731

   > Maybe when @michael-s-molina is in town we could do a committer meetup and discuss this and other similar topics? I'd love to talk about Flask blueprints and my vision for them. :)
   
   I would love that!


-- 
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 pull request #25196: feat: mechanism for re-saving charts

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

   > Thank you for the PR @betodealmeida. We have been discussing this internally at Airbnb and it's clear that these problems happen because we are missing schema versioning and validation for these types of fields. We have many bugs that were reported because these schemas change every time we introduce a new feature, essentially creating a new version of it, but we don't correctly support/migrate old versions. The fact that many of these schema transformations are on the frontend is really problematic because we end up creating columns in the database to store those transformations so they can be used by Python code when processing API requests as you can see by #15824, #15865, and #15846. @john-bodley Here's the motivation for the `params` and `query_context` duplication that haunts us 😄
   
   I totally agree, we need versioning, better separation of concerns between backend/frontend and query/presentation, as well as a simpler canonical data model for charts (which would also make it easier to develop new charts).
    
   > Another critical problem of that approach is that Superset assets can be consumed through APIs, without a frontend, and in that case, changes in feature flags don't reflect in the schemas returned by these APIs. We have 20 feature flags that will be removed in 4.0 (including `GENERIC_CHART_AXES`) which will ease the problem but we still need to discuss schema versioning for these types of fields.
   
   I agree here too, I've mentioned and been wanting to write a SIP proposing us to adopt blueprints, not only so it's easier to have multiple versions of an API at the same time, but we could even have blueprints for feature flags as well.
   
   > I understand the motivation for this PR but before we go down this road, is it possible to fix the issue using other means? Can you explain what breaks (video, stacktrace) and which chart types are affected?
   
   You and @geido probably know this better than me, but my understanding of the problem is that some legacy charts still require `granularity_sqla` to be present when `GENERIC_CHART_AXES` is present; other visualizations don't. We don't know which charts are in which group, @geido suggested trying to figure out programmatically but wasn't able to do that.
   
   From the errors I saw, there's no stacktrace, just a subtle silent error where **the chart query is generated incorrectly**. To the user, apparently no error happens, but the data presented might be fundamentally wrong.
   
   And to be clear, I don't think this is a road, I'm not proposing a new way of doing things. This is just a workaround allowing admins to fix some (or all) charts. This PR is a no-op unless the admin sets the flag to true in one or more charts. I don't think we should need this — which is why I called it a hack — and I hope we never need it again 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] michael-s-molina commented on pull request #25196: feat: mechanism for re-saving charts

Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on PR #25196:
URL: https://github.com/apache/superset/pull/25196#issuecomment-1708851288

   > But if we can indeed figure out a backend architecture that can facilitate arbitrary viz plugin logic without custom buildQueryContext hooks, this would indeed be a great simplification
   
   @villebro As always, I think there's a middle ground between visualization-specific code and business logic that needs to be supported by our APIs. I think this work is highly correlated with the new plugin architecture we were thinking about.


-- 
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] michael-s-molina commented on pull request #25196: feat: mechanism for re-saving charts

Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on PR #25196:
URL: https://github.com/apache/superset/pull/25196#issuecomment-1708864366

   > Can you share more about this? Can we have a bigger discussion?
   
   Sure. We can write a SIP or discussion thread about it after 3.0.


-- 
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 pull request #25196: feat: mechanism for re-saving charts

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

   > The other hand, I think we should try to avoid hacks too much in Superset, hacks have too much for now!
   
   I totally agree, but keep in mind that there are hacks and hacks. This PR is a hack in a sense that "we shouldn't have to do this", not in the sense of "this is really brittle and risky". As is, this PR does nothing, and the only thing it does is re-save the chart when the admin sets the column to 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] villebro commented on pull request #25196: feat: mechanism for re-saving charts

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

   > > I think in the long run we should really consider implementing a simple Node service that can render query context on the fly from form data (we could place this logic fairly easily in the websocket server). This would remove the need for persisting query context, which we can't update without resaving the chart if the `buildQuery` logic changes.
   > 
   > I don't know if it's possible, but what I'd like instead is a simpler contract between visualizations and the backend, so that this isn't even needed.
   
   @betodealmeida @michael-s-molina while there's definitely a lot of standardization that can be done, I think this will be very difficult to achieve. For example, the table chart triggers different queries if the pagination control is checked. Just looking at existing `buildQueryContext` hooks in the existing plugins shows that constructing chart data requests is non-trivial, and often very viz-specific. Therefore, moving this logic to the backend would be very difficult, as it would essentially require moving business logic into the backend. This was one of the main bottlenecks of `viz.py`, and one of the motivations for SIP-6: #5692. But if we can indeed figure out a backend architecture that can facilitate arbitrary viz plugin logic without custom `buildQueryContext` hooks, this would indeed be a great simplification


-- 
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 pull request #25196: feat: mechanism for re-saving charts

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

   Also, @Vitor-Avila raised a good point: this won't work if the user viewing chart doesn't have permission to overwrite the chart, so they might be looking at incorrect data until an admin opens the chart. I'm going to work on a different approach.


-- 
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] villebro commented on pull request #25196: feat: mechanism for re-saving charts

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

   @betodealmeida this would be great, I'd love to participate 👍 


-- 
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 pull request #25196: feat: mechanism for re-saving charts

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

   > Sure. We can write a SIP or discussion thread about it after 3.0.
   
   I think a discussion or even a meetup might be better. I have a lot of ideas and I know other people also have them, it would be nice to brainstorm before we have a proposal.


-- 
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 #25196: feat: mechanism for re-saving charts

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

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/25196?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   > Merging [#25196](https://app.codecov.io/gh/apache/superset/pull/25196?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (4416482) into [master](https://app.codecov.io/gh/apache/superset/commit/d0305445b2782264834821c3f1276d74c1b5f99f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (d030544) will **decrease** coverage by `10.37%`.
   > The diff coverage is `100.00%`.
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #25196       +/-   ##
   ===========================================
   - Coverage   69.09%   58.73%   -10.37%     
   ===========================================
     Files        1904     1904               
     Lines       74598    74602        +4     
     Branches     8246     8246               
   ===========================================
   - Hits        51544    43815     -7729     
   - Misses      20917    28650     +7733     
     Partials     2137     2137               
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/superset/pull/25196/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [hive](https://app.codecov.io/gh/apache/superset/pull/25196/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `53.75% <100.00%> (+<0.01%)` | :arrow_up: |
   | [mysql](https://app.codecov.io/gh/apache/superset/pull/25196/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [postgres](https://app.codecov.io/gh/apache/superset/pull/25196/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [presto](https://app.codecov.io/gh/apache/superset/pull/25196/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `53.65% <100.00%> (+<0.01%)` | :arrow_up: |
   | [python](https://app.codecov.io/gh/apache/superset/pull/25196/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.45% <100.00%> (-21.49%)` | :arrow_down: |
   | [sqlite](https://app.codecov.io/gh/apache/superset/pull/25196/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [unit](https://app.codecov.io/gh/apache/superset/pull/25196/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.27% <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=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Files Changed](https://app.codecov.io/gh/apache/superset/pull/25196?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/dashboard/actions/hydrate.js](https://app.codecov.io/gh/apache/superset/pull/25196?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9hY3Rpb25zL2h5ZHJhdGUuanM=) | `2.04% <ø> (ø)` | |
   | [.../src/dashboard/components/gridComponents/Chart.jsx](https://app.codecov.io/gh/apache/superset/pull/25196?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0NoYXJ0LmpzeA==) | `59.29% <ø> (ø)` | |
   | [...perset-frontend/src/dashboard/containers/Chart.jsx](https://app.codecov.io/gh/apache/superset/pull/25196?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0NoYXJ0LmpzeA==) | `75.00% <ø> (ø)` | |
   | [...t-frontend/src/explore/actions/saveModalActions.js](https://app.codecov.io/gh/apache/superset/pull/25196?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvYWN0aW9ucy9zYXZlTW9kYWxBY3Rpb25zLmpz) | `95.58% <ø> (ø)` | |
   | [...src/explore/components/ExploreChartPanel/index.jsx](https://app.codecov.io/gh/apache/superset/pull/25196?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FeHBsb3JlQ2hhcnRQYW5lbC9pbmRleC5qc3g=) | `77.63% <ø> (ø)` | |
   | [superset/charts/api.py](https://app.codecov.io/gh/apache/superset/pull/25196?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvY2hhcnRzL2FwaS5weQ==) | `50.32% <ø> (-37.50%)` | :arrow_down: |
   | [superset/charts/schemas.py](https://app.codecov.io/gh/apache/superset/pull/25196?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvY2hhcnRzL3NjaGVtYXMucHk=) | `99.38% <100.00%> (+<0.01%)` | :arrow_up: |
   | [superset/explore/schemas.py](https://app.codecov.io/gh/apache/superset/pull/25196?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvZXhwbG9yZS9zY2hlbWFzLnB5) | `100.00% <100.00%> (ø)` | |
   | [superset/models/slice.py](https://app.codecov.io/gh/apache/superset/pull/25196?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbW9kZWxzL3NsaWNlLnB5) | `59.11% <100.00%> (-28.52%)` | :arrow_down: |
   
   ... and [292 files with indirect coverage changes](https://app.codecov.io/gh/apache/superset/pull/25196/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