chore(ci): enabling A & B rules in Ruff #305
Conversation
leandrodamascena
left a comment
leandrodamascena
left a comment
There was a problem hiding this comment.
Hey @brnaba-aws I just left some comments explaining what I'm changing.
Thanks
| else: | ||
| if not isinstance(options.client, Anthropic): | ||
| raise ValueError("If streaming is disabled, the provided client must be an Anthropic client") | ||
| elif not isinstance(options.client, Anthropic): |
There was a problem hiding this comment.
Simplifying logic.
| elif not isinstance(options.client, Anthropic): | ||
| raise ValueError("If streaming is disabled, the provided client must be an Anthropic client") | ||
| self.client = options.client | ||
| elif self.streaming: |
There was a problem hiding this comment.
Simplifying logic.
| ) -> dict: | ||
| """Build the conversation command with all necessary configurations.""" | ||
| input = { | ||
| json_input = { |
There was a problem hiding this comment.
Avoiding variables names that shadow built-in python functions.
| async def _handle_streaming( | ||
| self, | ||
| input: dict, | ||
| payload_input: dict, |
There was a problem hiding this comment.
Avoiding parameters names that shadow built-in python functions.
| self, | ||
| streaming: bool, | ||
| input: dict, | ||
| payload_input: dict, |
There was a problem hiding this comment.
Avoiding parameters names that shadow built-in python functions.
| except Exception as error: | ||
| Logger.error(f"Error processing request with agent {agent.name}:{str(error)}") | ||
| raise f"Error processing request with agent {agent.name}:{str(error)}" | ||
| raise f"Error processing request with agent {agent.name}:{str(error)}" from error |
There was a problem hiding this comment.
Exceptions should propagate the original error for complete stack trace.
| f"{user_msg.role}:{user_msg.content[0].get('text','')}\n" | ||
| f"{asst_msg.role}:{asst_msg.content[0].get('text','')}\n" | ||
| for user_msg, asst_msg in zip(agents_history[::2], agents_history[1::2]) | ||
| for user_msg, asst_msg in zip(agents_history[::2], agents_history[1::2], strict=True) |
There was a problem hiding this comment.
Adding strict parameter to make sure zip function will unpack expected values OR will raise ValueError if not.
| session_id: str, | ||
| classifier_result: ClassifierResult, | ||
| additional_params: dict[str, str] = {}, | ||
| additional_params: dict[str, str] | None = None, |
There was a problem hiding this comment.
Should not add mutable parameters in the method signature. Moving the check to inside the function.
| session_id: str, | ||
| additional_params: dict[str, str] = {}, | ||
| stream_response: bool | None = False) -> AgentResponse: | ||
| async def route_request(self, user_input: str, user_id: str, session_id: str, additional_params: dict[str, str] | None = None, stream_response: bool | None = False) -> AgentResponse: |
There was a problem hiding this comment.
Should not add mutable parameters in the method signature. Moving the check to inside the function.
|
|
||
| properties = {} | ||
| for param_name, param in sig.parameters.items(): | ||
| for param_name, _param in sig.parameters.items(): |
There was a problem hiding this comment.
Variables is not being used, so, add _ to make it clear.
|
Closing as this has been ported manually due to conflict being too complex. |
2 participants
Issue Link (REQUIRED)
Fixes #304
Summary
Changes
This PR enables the following rule categories in our Ruff configuration:
A: Flake8-builtins - Checks for Python built-in names being used as variables/parametersB: Flake8-bugbear - Detects common bugs and design problemsA: Flake8-builtins Rules
These rules prevent shadowing of built-in names like
id,list,dict,input, etc. Shadowing built-ins can lead to:B: Flake8-bugbear Rules
Bugbear rules detect a wide range of common Python issues:
User experience
No changes in use experience
Checklist
If your change doesn't seem to apply, please leave them unchecked.
Is this a breaking change?
RFC issue number:
Checklist:
Acknowledgment
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.