Skip to content
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

fix(go/adbc/driver/flightsql): should use ctx.Err().Error() #1769

Merged
merged 1 commit into from
Apr 26, 2024

Conversation

cocoa-xu
Copy link
Contributor

As pointed out by @zeroshade here, this should be handled by doing adbc.Error{Msg: ctx.Err(), Code:....}.

@github-actions github-actions bot added this to the ADBC Libraries 1.0.0 milestone Apr 25, 2024
@zeroshade
Copy link
Member

@cocoa-xu looks like you need to update some of the python tests which were expecting specific error text results which are being changed by this

@cocoa-xu
Copy link
Contributor Author

Sorry but I didn't find there're any tests (in fact any code) that are expecting any error message that is (or contains) "Cancelled by request" or "Deadline exceeded".

Plus, even if there're some tests that should be updated because of the changes made here, it doesn't make sense that only Python 3.9 (and perhaps Python 3.11) failed on macos-13 instead of all of them (i.e., on macos-latest, ubuntu-latest and windows-latest for both Python 3.9 and 3.11). CI results.

Furthermore, the same test also failed in other CI runs, https://github.com/apache/arrow-adbc/actions/runs/8843931798/job/24285879645#step:14:1213, in PR #1762. It's unlikely that the changes in this PR has caused the above failed CI jobs.

@zeroshade zeroshade merged commit 59eede4 into apache:main Apr 26, 2024
37 of 39 checks passed
@lidavidm
Copy link
Member

@zeroshade there's something in the Flight SQL client that is treating io.EOF as an actual error, I think

cocoa-xu added a commit to meowcraft-dev/arrow-adbc that referenced this pull request May 8, 2024
…#1769)

As pointed out by @zeroshade
[here](apache#1722 (comment)),
this should be handled by doing `adbc.Error{Msg: ctx.Err(), Code:....}`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants