Repository navigation
Conversation
Code Review Agent Run #2d67aaActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
Sequence DiagramThis PR updates dashboard report URL generation so the force flag is always appended to permalink based report URLs. The report executor now propagates force_screenshot as force true or false, ensuring dashboard rendering follows the intended cache bypass behavior. sequenceDiagram
participant Scheduler
participant ReportExecutor
participant PermalinkService
participant DashboardWebApp
Scheduler->>ReportExecutor: Trigger dashboard report execution
ReportExecutor->>ReportExecutor: Read force screenshot setting
ReportExecutor->>PermalinkService: Create dashboard permalink key
PermalinkService-->>ReportExecutor: Return permalink key
ReportExecutor->>DashboardWebApp: Request permalink URL with force true or false
DashboardWebApp-->>ReportExecutor: Render dashboard using matching cache mode
Generated by CodeAnt AI |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #38691 +/- ##
==========================================
- Coverage 65.10% 64.29% -0.81%
==========================================
Files 1819 2532 +713
Lines 72670 129770 +57100
Branches 23222 29986 +6764
==========================================
+ Hits 47312 83441 +36129
- Misses 25358 44875 +19517
- Partials 0 1454 +1454
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR fixes dashboard report cache-bypass behavior by ensuring the force query parameter (driven by ReportSchedule.force_screenshot / “Ignore cache when generating report”) is propagated when dashboard report URLs are generated via the dashboard permalink/tab flow.
Changes:
- Add
force=true|falseto dashboard permalink URLs generated for tab/permalink-based dashboard report execution. - Update unit tests to assert the
forcequery param is present in generated dashboard permalink URLs (includingforce=falsewhen bypass is not enabled). - Add targeted unit test coverage for the
force=truepermalink case.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
superset/commands/report/execute.py |
Propagates force into Superset.dashboard_permalink URL generation for dashboard report permalink/tab rendering. |
tests/unit_tests/commands/report/execute_test.py |
Updates expectations and adds coverage to verify force is included in generated dashboard permalink URLs. |
You can also share your feedback on Copilot code review. Take the survey.
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
msyavuz
left a comment
There was a problem hiding this comment.
Thank you for fixing this one! Just a small nit
| """ | ||
| Get one tab url | ||
| """ | ||
| force = "true" if self._report_schedule.force_screenshot else "false" |
There was a problem hiding this comment.
I am wondering if we can dry this up a bit now with the same thing existing in L:219
There was a problem hiding this comment.
Hi @msyavuz , I'm working on resolving your suggestion but i have a quick question on local lint behavior.
I’m seeing a pre-commit/pylint mismatch: .pylintrc enables json-import, but superset/extensions/pylint.py only registers disallowed-sql-import and consider-using-transaction.
Local pylint then reports:
Unknown option value for '--enable' ... 'json-import'
I can reproduce this on current master, so it looks independent of this report fix.
Can you confirm the intended rule name/config here?
|
The diff shows adding a 'force' parameter in _get_tab_url, but without the full file or InlineCode for line 219, I can't verify if there's duplication to refactor. |
Code Review Agent Run #6de687Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
|
@ferjanin the fix is right, but two things before this can merge. First, it's conflicting with |
|
Happy to push some commits here if I can help this across the finish line :D |
|
@ferjanin still conflicting with master, and the dedup ask from March never landed. |
|
I can't push to this branch, so can't rebase for you... hope you can help get it across the finish line. |
|
Converting to draft... I might adopt this eventually, or we might close it if it remains inactive. |
|
This is a real, still-live bug — Seeing about adopting this one... |
|
No word from @ferjanin since March, and I still can't push to this branch (fork, no maintainer edits). The bug's still live on |
User description
fix(reports): propagate force flag in dashboard permalink report URLsSUMMARY
This PR fixes dashboard report cache bypass behavior when Ignore cache when generating report is enabled.
In the report execution flow, dashboard URLs generated through the permalink/tab path were not propagating the
forcequery parameter to the dashboard permalink URL. As a result, dashboard report rendering could proceed without the expected cache-bypass signal, unlike chart report flow whereforceis propagated correctly.This change updates dashboard permalink URL generation in report execution to include
force=true|falsebased onforce_screenshot, aligning dashboard reports with chart reports.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A (backend URL generation + unit-test coverage change)
TESTING INSTRUCTIONS
force=truein dashboard permalink flow.ADDITIONAL INFORMATION
forceto/api/v1/chart/datafor dashboard reports (cached results still used) #38672CodeAnt-AI Description
Include the cache-bypass setting in dashboard report links
What Changed
force=false; when it is on, they includeforce=true.Impact
✅ Correct cache-bypass behavior in dashboard reports✅ Fewer stale dashboard screenshots✅ Clearer report URL checks💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.