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

SNOW-956848 Store underlying error in SnowparkSQLException #1122

Merged
merged 8 commits into from
Nov 16, 2023

Conversation

sfc-gh-bchinn
Copy link
Collaborator

@sfc-gh-bchinn sfc-gh-bchinn commented Nov 1, 2023

  1. What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.

    https://snowflakecomputing.atlassian.net/browse/SNOW-956848

    This is mostly addressed by SNOW-947875 Add sql_error_code and raw_message into SnowparkSQLException #1111, but I think this is a better long-term solution, for the user to have access to any information in the underlying error.

  2. Fill out the following pre-review checklist:

    • I am adding a new automated test(s) to verify correctness of my new code
    • I am adding new logging messages
    • I am adding a new telemetry message
    • I am adding new credentials
    • I am adding a new dependency
  3. Please describe how your code solves the related issue.

@sfc-gh-bchinn sfc-gh-bchinn requested a review from a team as a code owner November 1, 2023 18:20
@sfc-gh-bchinn sfc-gh-bchinn force-pushed the bchinn-SNOW-956848-error branch 4 times, most recently from 2268924 to c772e7c Compare November 2, 2023 00:32
Comment on lines -242 to +255
f"DataFrame alias unrecognized. A subset of columns corresponding to Dataframe alias '{alias}' can not be found. "
"1208",
f"DataFrame alias unrecognized. A subset of columns corresponding to Dataframe alias '{alias}' can not be found. ",
error_code="1208",
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😱 This almost makes me wish Python didn't allow implicit string concatenation

@sfc-gh-bchinn sfc-gh-bchinn force-pushed the bchinn-SNOW-956848-error branch from c772e7c to 10fd7cd Compare November 2, 2023 00:37
Copy link
Collaborator

@sfc-gh-aalin sfc-gh-aalin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, this looks much cleaner, thanks for the changes!

@sfc-gh-aalin
Copy link
Collaborator

sfc-gh-aalin commented Nov 2, 2023

Adding @sfc-gh-aling as a reviewer since I'm not a code owner.

It looks like the two of us had the same idea :)

@sfc-gh-bchinn sfc-gh-bchinn force-pushed the bchinn-SNOW-956848-error branch from 10fd7cd to b07de12 Compare November 3, 2023 16:26
@sfc-gh-bchinn
Copy link
Collaborator Author

@sfc-gh-yixie @sfc-gh-sfan can one of you merge?

@sfc-gh-sfan
Copy link
Collaborator

There is a CLA bot thingy that I'll check with the team and see what to do about it.

@sfc-gh-bchinn
Copy link
Collaborator Author

@sfc-gh-sfan any updates on this?

CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Contributor

@sfc-gh-aalam sfc-gh-aalam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Outside of changelog, other changes look good to me

@sfc-gh-bchinn sfc-gh-bchinn force-pushed the bchinn-SNOW-956848-error branch from b07de12 to 28b9541 Compare November 9, 2023 23:10
@sfc-gh-bchinn sfc-gh-bchinn force-pushed the bchinn-SNOW-956848-error branch from 28b9541 to 67c4c01 Compare November 16, 2023 00:51
@sfc-gh-bchinn
Copy link
Collaborator Author

@sfc-gh-aalin @sfc-gh-sfan Can one of you merge? I don't have write access

@sfc-gh-aalin
Copy link
Collaborator

@sfc-gh-aalin @sfc-gh-sfan Can one of you merge? I don't have write access

Alas, neither do I

@sfc-gh-sfan sfc-gh-sfan merged commit 3ffe288 into main Nov 16, 2023
51 checks passed
@sfc-gh-sfan sfc-gh-sfan deleted the bchinn-SNOW-956848-error branch November 16, 2023 20:05
@github-actions github-actions bot locked and limited conversation to collaborators Nov 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants