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

SQL: Set relevant errors to downstream #1159

Merged
merged 2 commits into from
Dec 5, 2024

Conversation

aangelisc
Copy link
Contributor

@aangelisc aangelisc commented Nov 28, 2024

Making some changes based on some errors being incorrectly marked in the MSSQL data source (that I believe can also be incorrectly marked in Postgres and MySQL also).

The errors that I've modified here are ones that I believe can only error due to downstream issues rather than the plugin. Please let me know if you disagree as I think determining which the ultimate source of an error is quite challenging.

With some follow up PR's to grafana/grafana this would fix some issues we're seeing atm.

If we don't want to mark all of these as downstream then I believe we should at least mark the rows.Err() error as downstream.

@aangelisc aangelisc requested a review from a team November 28, 2024 18:39
@aangelisc aangelisc self-assigned this Nov 28, 2024
@aangelisc aangelisc requested review from bossinc, adamyeats and katebrenner and removed request for a team November 28, 2024 18:39
@aangelisc aangelisc requested a review from a team as a code owner November 28, 2024 18:39
@aangelisc aangelisc requested a review from alyssabull November 28, 2024 18:39
Copy link
Member

@zoltanbedi zoltanbedi left a comment

Choose a reason for hiding this comment

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

I'd only mark the rows.Err() as downstream but lgtm otherwise.

@@ -22,7 +23,7 @@ import (
func FrameFromRows(rows *sql.Rows, rowLimit int64, converters ...Converter) (*data.Frame, error) {
types, err := rows.ColumnTypes()
if err != nil {
return nil, err
return nil, backend.DownstreamError(err)
Copy link
Member

Choose a reason for hiding this comment

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

I haven't seen this one erroring. I probably wouldn't change this to downstream. The same for rows.Columns as it retruns the same errors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated! thank you @zoltanbedi

@@ -73,7 +74,7 @@ func FrameFromRows(rows *sql.Rows, rowLimit int64, converters ...Converter) (*da
}

if err := rows.Err(); err != nil {
return frame, err
return frame, backend.DownstreamError(err)
Copy link
Member

Choose a reason for hiding this comment

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

I'm fine marking this as a downstream error.

@aangelisc aangelisc merged commit 3ecf138 into main Dec 5, 2024
3 checks passed
@aangelisc aangelisc deleted the andreas/sql-errorsource-updates branch December 5, 2024 12:02
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.

2 participants