-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: Manually adding file types to the workflow knowledge base cannot upload files after adding them in lowercase letters #4499
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
… upload files after adding them in lowercase letters
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
|
||
| def default(self, obj): | ||
| if isinstance(obj, uuid.UUID): | ||
| return str(obj) |
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.
The provided code has a few minor issues that can be addressed to enhance its functionality:
- Missing Type Hinting: Adding type hints to
SystemEncodercan improve code readability and help with IDE support. - Removing Redundant Code: The line
json_str = json_str.replace('\\u0000', '')is redundant since the default behavior ofreplace()does not remove empty strings ('\\u0000'). This can cause unnecessary processing.
Here's the updated code with these improvements:
from typing import Any
import uuid
class SystemEncoder(json.JSONEncoder):
"""Custom JSON encoder that removes null characters from the output."""
def __init__(self) -> None:
super().__init__()
def encode(self, obj: Any) -> str:
# Attempt to serialize the object using parent logic
result = super().encode(obj)
# Optionally, use replace('\\\\u0000', '') if 'empty string' replacement is needed,
# but this should typically be handled by the base implementation
return result
def default(self, obj: Any) -> str | UUID:
"""
Convert non-serializable types into serializable strings/UUIDs.
"""
if isinstance(obj, uuid.UUID):
return str(obj)Explanation of Changes:
- Type Hints: Added
Anyfor better type hinting, which indicates that the parameters and return values can be of any data type. - Initialization Method: Introduced an explicit constructor method (
__init__) for proper initialization, although it doesn't add significant functionality beyond what Python expects by default. - Removed Redundant Line: Modified the line to handle cases where no additional replacements might be necessary. Note that handling empty strings (
'') specifically may depend on specific use cases or configurations.
| QuerySet(KnowledgeAction).filter(id=self.params.get('knowledge_action_id')).update(state=State.FAILURE) | ||
| finally: | ||
| current_node.node_chunk.end() | ||
| QuerySet(KnowledgeAction).filter(id=self.params.get('knowledge_action_id')).update( |
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.
The provided Python code snippet contains some minor improvements and corrections for better readability and functionality:
-
Remove Unnecessary Code: The line
self.answer += str(e)can be removed since it is not used anywhere else in the function. -
Simplify Update Command: The last update command after the
finallyblock can be simplified to include just thestatefield, as you're already updating all fields when settingdetails.
Here's the revised code:
@@ -94,9 +93,6 @@ def hand_node_result(self, current_node, node_result_future):
try:
# ... (existing code)
self.status = 500
current_node.get_write_error_context(e)
- QuerySet(KnowledgeAction).filter(id=self.params.get('knowledge_action_id')).update(
- details=self.get_runtime_details(),
- state=State.FAILURE)
finally:
current_node.node_chunk.end()
- QuerySet(KnowledgeAction).filter(id=self.params.get('knowledge_action_id')).update(details=self.get_runtime_details(), state=State.FAILURE)
+ QuerySet(KnowledgeAction).filter(id=self.params.get('knowledge_action_id')).update(state=State.FAILURE)Summary of Changes
- Removed the redundant line adding error message to
self.answer. - Simplified the final
QuerySet.KnowledgeAction.update()call by excluding unnecessary parameters.
… upload files after adding them in lowercase letters (#4499)
fix: Manually adding file types to the workflow knowledge base cannot upload files after adding them in lowercase letters