-
Notifications
You must be signed in to change notification settings - Fork 199
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
Conversation
…#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.
There was a problem hiding this 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 anSDKError
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 fromerror
toerror_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.
- Corrected variable name in
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
-
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. ↩
There was a problem hiding this 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 overself.errors
, where each item is a singleErrorDetail
instance. While the current nameerror_details
successfully resolves the bug, renaming it toerror_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.
… (#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
toerror_details
to:__str__
implementation.Notes
error_details
represents a single instance; per PEP 8, we may later rename it toerror_detail
for readability.