-
Notifications
You must be signed in to change notification settings - Fork 13.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: Refactor ExploreMixin to power both Datasets (SqlaTable) and Query models #22853
Conversation
Pinging @villebro since I know he's keen on this. Might wanna fill out the PR description though, for the rest of the world ;) |
Codecov Report
@@ Coverage Diff @@
## master #22853 +/- ##
==========================================
+ Coverage 67.70% 67.94% +0.23%
==========================================
Files 1918 1918
Lines 74133 73873 -260
Branches 8052 8052
==========================================
- Hits 50193 50190 -3
+ Misses 21887 21630 -257
Partials 2053 2053
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 3 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
36346e1
to
2b1414c
Compare
743a478
to
3d7ae3d
Compare
/test_env up |
/testenv up |
@hughhhh Ephemeral environment spinning up at http://34.221.152.250:8080. Credentials are |
3d7ae3d
to
47c2f83
Compare
issue 4: issue 5:
aggregate.mp4issue 6: User is not able to create dataset when chart = power query. create.dataset.mp4Also user can't create chart using chart power query (the same error^). save.chart.mp4 |
Ephemeral environment shutdown and build artifacts deleted. |
…e) and Query models (apache#22853)" This reverts commit ba7b6fd.
…e) and Query models (apache#22853)" This reverts commit d581d69.
timestamp = dttm_col.get_timestamp_expression( | ||
time_grain=time_grain, template_processor=template_processor | ||
) | ||
timestamp = dttm_col.get_timestamp_expression( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SUMMARY
Cleaning up tech debt from intial SIP-68 work to move towards having newer models (Table, Dataset) to run in explore. Since we are deprioritizing that work, refactoring the ExploreMixin to power
SqlaTable
andQuery
which are legacy models. Main reason is to have one codepath on executing queries in explore vs. having divergent paths at the moment.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION