Skip to content

BugFix(SDKError): Correct error formatting in SDKError __str__ method… #1588

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

Merged
merged 1 commit into from
May 27, 2025

Conversation

drstrangelooker
Copy link
Collaborator

… (#1587)

BugFix: Correct error formatting in SDKError.str

Problem

Calling str(SDKError) raised the following runtime error: looker_sdk.error.SDKError: <exception str() failed>

This happened when the SDK attempted to convert the error to a string, which failed due to a malformed generator expression or naming conflict during the formatting of self.errors.

Fix

Updated the loop variable in the generator expression from error to error_details to:

  • Avoid potential name shadowing.
  • Improve clarity and correctness of the __str__ implementation.
  • Resolve the runtime error so the error can now be printed correctly.

Notes

  • The variable error_details represents a single instance; per PEP 8, we may later rename it to error_detail for readability.

…#1587)

### BugFix: Correct error formatting in SDKError.__str__

#### Problem
Calling `str(SDKError)` raised the following runtime error:
`looker_sdk.error.SDKError: <exception str() failed>`

This happened when the SDK attempted to convert the error to a string,
which failed due to a malformed generator expression or naming conflict
during the formatting of `self.errors`.

#### Fix
Updated the loop variable in the generator expression from `error` to
`error_details` to:
- Avoid potential name shadowing.
- Improve clarity and correctness of the `__str__` implementation.
- Resolve the runtime error so the error can now be printed correctly.

#### Notes
- The variable `error_details` represents a single instance; per PEP 8,
we may later rename it to `error_detail` for readability.
@drstrangelooker drstrangelooker requested a review from a team as a code owner May 27, 2025 15:38
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Hello @drstrangelooker, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

Hello, Gemini/gemini-code-assist here to provide a summary of this pull request. This PR addresses a bug where attempting to get the string representation of an SDKError instance would fail with a runtime error (<exception str() failed>). The root cause was identified as a potential variable naming conflict or shadowing within the generator expression used to format the error details in the __str__ method of the SDKError class. The fix involves renaming the loop variable in this generator expression to resolve the conflict and ensure the error details can be correctly formatted into the final string output.

Highlights

  • Bug Fix: Resolves a runtime error that occurred when calling str() on an SDKError instance, which prevented the error details from being properly formatted and displayed.
  • Code Change: Updates the loop variable name within the generator expression inside the SDKError.__str__ method from error to error_details to avoid a naming conflict and fix the string formatting issue.

Changelog

  • python/looker_sdk/error.py
    • Corrected variable name in SDKError.__str__ generator expression (error -> error_details) to fix error formatting bug.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.


A variable's name,
Caused the error string to fail,
Renamed, now it works.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request effectively addresses a runtime error in the SDKError.__str__ method. The problem, as described, where str(SDKError) would fail due to an issue in formatting self.errors (likely a name shadowing conflict with the loop variable error), is resolved by renaming the loop variable to error_details.

This is a precise and important fix, as correct error reporting is crucial for SDK usability. The code change is clear and directly tackles the identified issue. Well done on diagnosing and fixing this bug!

The PR description is also very clear and helpfully includes a note about a potential future refinement for the loop variable name (error_details to error_detail) to align even better with PEP 8, which is a good observation.

Summary of Findings

  • Loop Variable Naming Convention (PEP 8): The loop variable error_details is used to iterate over self.errors, where each item is a single ErrorDetail instance. While the current name error_details successfully resolves the bug, renaming it to error_detail (singular) would align more closely with common Python (PEP 8) naming conventions for loop variables that represent a single item from a collection. This is a low-severity stylistic observation. It's excellent that this was already noted in the pull request description as a potential future refinement. Due to the review settings focusing on medium severity issues and above, a formal review comment was not added for this point.

Merge Readiness

The pull request addresses a significant bug and the fix is correct and well-implemented. The minor stylistic point regarding variable naming is already acknowledged by the author. The code is in good shape and ready for merging. As a reviewer, I cannot approve the pull request, but I recommend it for merging once any internal review processes are complete.

Copy link
Contributor

Python Tests

 10 files  ±0   10 suites  ±0   1m 20s ⏱️ -1s
144 tests ±0  139 ✅ ±0   5 💤 ±0  0 ❌ ±0 
792 runs  ±0  770 ✅ ±0  22 💤 ±0  0 ❌ ±0 

Results for commit 120456d. ± Comparison against base commit 8e80ac5.

@drstrangelooker drstrangelooker merged commit 4794aab into main May 27, 2025
26 checks passed
@drstrangelooker drstrangelooker deleted the lookerErrorLogs branch May 27, 2025 17:42
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