You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "sfirke (via GitHub)" <gi...@apache.org> on 2024/03/08 18:34:01 UTC
[PR] fix(drilling): drill by pagination works with MSSQL data source [superset]
sfirke opened a new pull request, #27442:
URL: https://github.com/apache/superset/pull/27442
### SUMMARY
fixes #24072
### TESTING INSTRUCTIONS
- CI
- Sam is going to deploy and test the docker image generated by this PR
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [x] Has associated issue: #24072
--
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
Re: [PR] DON'T MERGE - fix(drilling): drill by pagination works with MSSQL data source [superset]
Posted by "john-bodley (via GitHub)" <gi...@apache.org>.
john-bodley commented on PR #27442:
URL: https://github.com/apache/superset/pull/27442#issuecomment-1989224902
@sfirke once this is ready for review would you mind adding the relevant reviewers?
--
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
Re: [PR] fix(drilling): drill by pagination works with MSSQL data source [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #27442:
URL: https://github.com/apache/superset/pull/27442#issuecomment-2738778205
/korbit-review
--
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
Re: [PR] fix(drilling): drill by pagination works with MSSQL data source [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #27442:
URL: https://github.com/apache/superset/pull/27442#issuecomment-1997777491
I wonder, would it work on your fork if you added your own dockerhub secrets/credentials to your own github fork?
--
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
Re: [PR] fix(drilling): drill by pagination works with MSSQL data source (still testing) [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #27442:
URL: https://github.com/apache/superset/pull/27442#issuecomment-1994946841
I am seeing the Docker / docker-build (ci) step is succeeding, but I don't see a corresponding image on Docker Hub: https://hub.docker.com/r/apache/superset/tags Am I missing it there when I look + search, or is it not pushing to Docker Hub? It seems like other PR images are. That would allow me to deploy the image and test it thoroughly.
--
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
Re: [PR] DON'T MERGE - fix(drilling): drill by pagination works with MSSQL data source [superset]
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on PR #27442:
URL: https://github.com/apache/superset/pull/27442#issuecomment-1992244531
@sfirke the CI error you're getting
`superset/common/query_actions.py:156: error: List item 0 has incompatible type "Tuple[Union[AdhocColumn, str], bool]"; expected "Tuple[Union[AdhocMetric, str], bool]" [list-item]`
is because you're passing a `Column` here
query_obj.orderby = [(query_obj.columns[0], True)]
instead of a `Metric` which is what `OrderBy` expects.
```
Column = Union[AdhocColumn, str]
Metric = Union[AdhocMetric, str]
OrderBy = tuple[Metric, bool]
```
--
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
Re: [PR] fix(drilling): drill by pagination works with MSSQL data source [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on PR #27442:
URL: https://github.com/apache/superset/pull/27442#issuecomment-1997859679
`supersetbot` acts as a neat wrapper around `docker build` and could be expanded to support other hub repos. The main win from using the wrapper is setting the read cache to the location used by `master` builds, meaning you potentially have a lot less to build locally. It also has a notion of build "presets" that point to combination of layers and settings.
```
$ nvm use 20
$ npm i -g supersetbot
$ supersetbot docker --help
Usage: supersetbot docker [options]
Generates/run docker build commands use in CI
Options:
-t, --preset Build preset
-c, --context <context> Build context (default: "local")
-r, --context-ref <ref> Reference to the PR, release, or branch
-p, --platform <platform...> Platforms (multiple values allowed)
-f, --force-latest Force the "latest" tag on the release
-v, --verbose Print more info
-h, --help display help for command
$ supersetbot docker --dry-run
docker buildx build \
--load \
-t apache/superset:8ed368c1fe624e6288d9270997098732d2289f7c-arm \
-t apache/superset:8ed368c-arm \
--cache-from=type=registry,ref=apache/superset-cache:3.9-slim-bookworm \
\
--target lean \
--build-arg PY_VER=3.9-slim-bookworm \
--platform linux/arm64 \
--label sha=8ed368c1fe624e6288d9270997098732d2289f7c \
--label target=lean \
--label build_trigger= \
--label base=3.9-slim-bookworm \
--label build_actor=undefined \
.
```
notice the `--cache-from`
--
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
Re: [PR] fix(drilling): drill by pagination works with MSSQL data source (still testing) [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #27442:
URL: https://github.com/apache/superset/pull/27442#issuecomment-1994973222
I think that's correct, that PRs build the docker image (as a bit of a smoke test) but don't push them to dockerhub (to reduce clutter). I think @mistercrunch set up a command in his recent work to manually publish a branch's docker image, but I can't recall what it is :)
--
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
Re: [PR] fix(drilling): drill by pagination works with MSSQL data source [superset]
Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.
korbit-ai[bot] commented on code in PR #27442:
URL: https://github.com/apache/superset/pull/27442#discussion_r2004715307
##########
superset/common/query_actions.py:
##########
@@ -180,6 +179,7 @@ def _get_drill_detail(
else:
qry_obj_cols.append(o.column_name)
query_obj.columns = qry_obj_cols
+ query_obj.orderby = [(query_obj.columns[0], True)]
Review Comment:
### Unsafe Column List Access <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The code assumes `query_obj.columns` is non-empty when setting the orderby clause, which could lead to an IndexError if columns list is empty.
###### Why this matters
If no columns are present in the datasource or query object, accessing index 0 will crash the application with an IndexError.
###### Suggested change ∙ *Feature Preview*
Add a guard clause to check for columns before setting the orderby:
```python
if query_obj.columns:
query_obj.orderby = [(query_obj.columns[0], True)]
else:
query_obj.orderby = []
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8afedbab-8c65-44d8-8101-b9ed8276fa1a/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8afedbab-8c65-44d8-8101-b9ed8276fa1a?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8afedbab-8c65-44d8-8101-b9ed8276fa1a?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8afedbab-8c65-44d8-8101-b9ed8276fa1a?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8afedbab-8c65-44d8-8101-b9ed8276fa1a)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:ab597ba6-e466-48e0-9902-421d316fdd84 -->
[](ab597ba6-e466-48e0-9902-421d316fdd84)
--
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
Re: [PR] fix(drilling): drill by pagination works with MSSQL data source [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #27442:
URL: https://github.com/apache/superset/pull/27442#issuecomment-3118727584
@rusackas yes, alas. I will still get to this _sometime_, but not imminently.
--
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
Re: [PR] fix(drilling): drill by pagination works with MSSQL data source [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on PR #27442:
URL: https://github.com/apache/superset/pull/27442#issuecomment-1996762144
@sfirke as far as I know there isn't a way to do this in this context (PR from a fork) because of potential security issues. Docker creds just aren't available in this context, if they were, an attacker could push arbitrary code to the `latest` image for instance.
You should be able to `docker build` locally at will though, any reason why you want CI to build it for you?
--
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
Re: [PR] DON'T MERGE - fix(drilling): drill by pagination works with MSSQL data source [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on code in PR #27442:
URL: https://github.com/apache/superset/pull/27442#discussion_r1521969160
##########
superset/common/query_actions.py:
##########
@@ -154,6 +153,7 @@ def _get_samples(
qry_obj_cols.append(o.get("column_name"))
else:
qry_obj_cols.append(o.column_name)
+ query_obj.orderby = [(query_obj.columns[0], True)]
Review Comment:
Thanks for the feedback @michael-s-molina 🙏 ! Someone in the associated issue thread said this one-liner was a solution for them so I hoped it would be this simple but it looks like not.
@zhaoyongjie gave a [high-level description of how he thinks this should be fixed](https://github.com/apache/superset/issues/24072#issuecomment-1859252019). Implementing it is outside of my current understanding unfortunately.
--
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
Re: [PR] fix(drilling): drill by pagination works with MSSQL data source [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke closed pull request #27442: fix(drilling): drill by pagination works with MSSQL data source
URL: https://github.com/apache/superset/pull/27442
--
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
Re: [PR] DON'T MERGE - fix(drilling): drill by pagination works with MSSQL data source [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #27442:
URL: https://github.com/apache/superset/pull/27442#issuecomment-1986216341
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/27442?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 59.68%. Comparing base [(`ce0b70c`)](https://app.codecov.io/gh/apache/superset/commit/ce0b70cc8685aa4d83c0c4ad0fb8f03fb7e28ede?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`b17d3fd`)](https://app.codecov.io/gh/apache/superset/pull/27442?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 2 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #27442 +/- ##
==========================================
- Coverage 67.34% 59.68% -7.67%
==========================================
Files 1909 1909
Lines 74622 74622
Branches 8324 8324
==========================================
- Hits 50257 44538 -5719
- Misses 22312 28031 +5719
Partials 2053 2053
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/27442/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [mysql](https://app.codecov.io/gh/apache/superset/pull/27442/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/27442/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/27442/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `53.70% <100.00%> (?)` | |
| [python](https://app.codecov.io/gh/apache/superset/pull/27442/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `62.34% <100.00%> (-15.90%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/27442/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/27442/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.54% <0.00%> (?)` | |
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.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/27442?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?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
Re: [PR] fix(drilling): drill by pagination works with MSSQL data source [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #27442:
URL: https://github.com/apache/superset/pull/27442#issuecomment-2738777992
Looks like this is pretty stuck. Maybe it needs a rebase? I'll convert it to draft for now, but mark it as ready if/when you think it might be.
--
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
Re: [PR] fix(drilling): drill by pagination works with MSSQL data source [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #27442:
URL: https://github.com/apache/superset/pull/27442#issuecomment-3113997255
@sfirke is this still WIP?
--
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
Re: [PR] fix(drilling): drill by pagination works with MSSQL data source [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #27442:
URL: https://github.com/apache/superset/pull/27442#issuecomment-1997433076
That makes sense, thanks for explaining how it works & why those PR images don't get pushed.
Re: motive, one time my local setup was messed up and docker build was failing so I deployed the image from docker hub and got in that habit. But now I have repaired my local setup and will do as you suggest.
--
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
Re: [PR] fix(drilling): drill by pagination works with MSSQL data source [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #27442:
URL: https://github.com/apache/superset/pull/27442#issuecomment-2738957077
"Draft" sounds right, I'd like to see this through but won't get to it
anytime soon
On Wed, Mar 19, 2025, 9:26 PM korbit-ai[bot] ***@***.***>
wrote:
> ***@***.***[bot]* commented on this pull request.
>
> I've completed my review and didn't find any issues.
> Suppressed issues based on your team's Korbit activity
> This issue Is similar to Because
>
> line 182
> <https://github.com/apache/superset/pull/27442/files/908a3aab440ee1252a12cde2bf6d1e05b1cf223c..a995f08def74a6c86495288a2001b3bb435fe8ec#diff-7b0f273ec0cd03f2e1c6d3822dbf3573dfcd5c08635ae246a93fe371899bf048R182-R182>
> :
>
> The code assumes query_obj.columns will always have at least one element,
> which could lead to an IndexError if the columns list is empty.
>
> Missing empty array check for queriesResponse
> <https://github.com/apache/superset/pull/32025#discussion_r1933775498>
> Similar issues were not addressed in the past
>
> When you react to issues (for example, an upvote or downvote) or you fix
> them, Korbit will tune future reviews based on these signals.
> Files scanned
> File Path Reviewed
> superset/superset_typing.py ✅
> superset/common/query_actions.py ✅
>
> Explore our documentation <https://docs.korbit.ai> to understand the
> languages and file types we support
> <https://docs.korbit.ai/configuration/supported-languages> and the files
> we ignore <https://docs.korbit.ai/configuration/ignoring-files>.
>
> Need a new review? Comment /korbit-review on this PR and I'll review your
> latest changes.
> Korbit Guide: Usage and Customization Interacting with Korbit
>
> - You can manually ask Korbit to review your PR using the
> /korbit-review command in a comment at the root of your PR.
> - You can ask Korbit to generate a new PR description using the
> /korbit-generate-pr-description command in any comment on your PR.
> - Too many Korbit comments? I can resolve all my comment threads if
> you use the /korbit-resolve command in any comment on your PR.
> - On any given comment that Korbit raises on your pull request, you
> can have a discussion with Korbit by replying to the comment.
> - Help train Korbit to improve your reviews by giving a 👍 or 👎 on
> the comments Korbit posts.
>
> Customizing Korbit
>
> - Check out our docs
> <https://docs.korbit.ai/configuration/organization-settings> on how
> you can make Korbit work best for you and your team.
> - Customize Korbit for your organization through the Korbit Console
> <https://app.korbit.ai/aa91ff46-6083-4491-9416-b83dd1994b51/settings>.
>
> Current Korbit Configuration General Settings
> Setting Value
> Review Schedule Automatic excluding drafts
> Max Issue Count 10
> Automatic PR Descriptions ❌ Issue Categories
> Category Enabled
> Documentation ✅
> Logging ✅
> Error Handling ✅
> Readability ✅
> Design ✅
> Performance ✅
> Security ✅
> Functionality ✅ Feedback and Support
>
> - Tell us what you think of Korbit
> <https://rfn13ctpmt0.typeform.com/to/DOoeLyoN>
> - Schedule a call with our team
> <https://meetings.salesloft.com/korbittechnologies/korbitai>
> - Email us @ ***@***.***
>
> Note
>
> Korbit Pro is free for open source projects 🎉
> Looking to add Korbit to your team? Get started with a free 2 week trial
> here <https://www.korbit.ai?utm_source=github_open_source_messaging>
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/superset/pull/27442#pullrequestreview-2700815804>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ABZYDEEFZICSAZBRP3BPHGL2VIKKVAVCNFSM6AAAAABZMHSTAOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDOMBQHAYTKOBQGQ>
> .
> You are receiving this because you modified the open/close state.Message
> ID: ***@***.***>
>
--
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
Re: [PR] DON'T MERGE - fix(drilling): drill by pagination works with MSSQL data source [superset]
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on code in PR #27442:
URL: https://github.com/apache/superset/pull/27442#discussion_r1521921799
##########
superset/common/query_actions.py:
##########
@@ -154,6 +153,7 @@ def _get_samples(
qry_obj_cols.append(o.get("column_name"))
else:
qry_obj_cols.append(o.column_name)
+ query_obj.orderby = [(query_obj.columns[0], True)]
Review Comment:
I don't have much context here but given that we're explicitly setting:
```
query_obj.orderby = []
query_obj.metrics = None
```
and that the definition of `OrderBy` is:
```
OrderBy = tuple[Metric, bool]
```
Setting the order by to the first column might not be the correct answer given that it only accepts ordering by metric which is explicitly being set to empty. Maybe @zhaoyongjie or @villebro have more context and can help us.
--
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
Re: [PR] fix(drilling): drill by pagination works with MSSQL data source [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on code in PR #27442:
URL: https://github.com/apache/superset/pull/27442#discussion_r1523858199
##########
superset/common/query_actions.py:
##########
@@ -154,6 +153,7 @@ def _get_samples(
qry_obj_cols.append(o.get("column_name"))
else:
qry_obj_cols.append(o.column_name)
+ query_obj.orderby = [(query_obj.columns[0], True)]
Review Comment:
Evan suggested changing the definition of OrderBy and that seems to have worked.
--
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
Re: [PR] fix(drilling): drill by pagination works with MSSQL data source [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #27442:
URL: https://github.com/apache/superset/pull/27442#issuecomment-3161724668
@rusackas I have superseded this pull request with a new version, #34583, that is passing CI. It needs a review but maybe we can get this into 6.0 and 5.0.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
Re: [PR] fix(drilling): drill by pagination works with MSSQL data source [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke closed pull request #27442: fix(drilling): drill by pagination works with MSSQL data source
URL: https://github.com/apache/superset/pull/27442
--
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
Re: [PR] fix(drilling): drill by pagination works with MSSQL data source (still testing) [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #27442:
URL: https://github.com/apache/superset/pull/27442#issuecomment-1995112813
Ah indeed, the PR whose image I see on Docker Hub is one from @mistercrunch: https://github.com/apache/superset/pull/27465 Max, is there a command I can use to push a PR branch image to Docker Hub?
--
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
Re: [PR] fix(drilling): drill by pagination works with MSSQL data source [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #27442:
URL: https://github.com/apache/superset/pull/27442#issuecomment-2000409843
I'm stuck, it's unclear to me why this test is failing:
```
=================================== FAILURES ===================================
___________ TestPostChartDataApi.test_chart_data_applied_time_extras ___________
self = <tests.integration_tests.charts.data.api_tests.TestPostChartDataApi testMethod=test_chart_data_applied_time_extras>
@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
def test_chart_data_applied_time_extras(self):
"""
Chart data API: Test chart data query with applied time extras
"""
self.query_context_payload["queries"][0]["applied_time_extras"] = {
"__time_range": "100 years ago : now",
"__time_origin": "now",
}
rv = self.post_assert_metric(CHART_DATA_URI, self.query_context_payload, "data")
> self.assertEqual(rv.status_code, 200)
E AssertionError: 400 != 200
```
And why is `test-postgres-presto` passing but `test-postgres-hive` not?
--
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
Re: [PR] fix(drilling): drill by pagination works with MSSQL data source [superset]
Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.
korbit-ai[bot] commented on PR #27442:
URL: https://github.com/apache/superset/pull/27442#issuecomment-2738957298
@sfirke I am looking at your pull request. The description will be updated shortly. In the meantime, please do not edit the description until I have finished writing mine.
[](27fd7502-26f1-407f-89aa-e143732f60c0)
--
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