-
Notifications
You must be signed in to change notification settings - Fork 0
test: check how copilot review pr #3
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
base: main
Are you sure you want to change the base?
Changes from all commits
4c7d795
e2e515b
ab75ecd
0c18573
bb53688
bed9e92
f706e5b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -25,31 +25,37 @@ def __init__( | |||||||||||||||||||||
) | ||||||||||||||||||||||
except exceptions.CosmosHttpResponseError as e: | ||||||||||||||||||||||
if e.status_code == 401: | ||||||||||||||||||||||
raise ValueError("Invalid credentials") from e | ||||||||||||||||||||||
raise ValueError("Unauthorized") from e | ||||||||||||||||||||||
else: | ||||||||||||||||||||||
raise ValueError("Invalid CosmosDB endpoint") from e | ||||||||||||||||||||||
logging.error("Cosmos error: %s", e) | ||||||||||||||||||||||
raise ValueError("Something went wrong!") from e | ||||||||||||||||||||||
|
||||||||||||||||||||||
try: | ||||||||||||||||||||||
self.database_client = self.cosmosdb_client.get_database_client( | ||||||||||||||||||||||
database_name | ||||||||||||||||||||||
) | ||||||||||||||||||||||
except exceptions.CosmosResourceNotFoundError: | ||||||||||||||||||||||
raise ValueError("Invalid CosmosDB database name") | ||||||||||||||||||||||
except Exception as e: | ||||||||||||||||||||||
logging.error("Unexpected error while accessing CosmosDB database: %s", e) | ||||||||||||||||||||||
raise | ||||||||||||||||||||||
|
||||||||||||||||||||||
try: | ||||||||||||||||||||||
self.container_client = self.database_client.get_container_client( | ||||||||||||||||||||||
container_name | ||||||||||||||||||||||
) | ||||||||||||||||||||||
except exceptions.CosmosResourceNotFoundError: | ||||||||||||||||||||||
raise ValueError("Invalid CosmosDB container name") | ||||||||||||||||||||||
except: | ||||||||||||||||||||||
pass | ||||||||||||||||||||||
Comment on lines
+49
to
+50
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid using a bare except block that swallows exceptions without logging; consider catching specific exceptions to better handle error scenarios.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback
Comment on lines
+49
to
+50
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using a bare 'except:' block here may silently mask errors during container client initialization. Consider catching 'Exception as e' and logging the error to aid in debugging.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||||||||||||||
|
||||||||||||||||||||||
async def ensure(self): | ||||||||||||||||||||||
if ( | ||||||||||||||||||||||
not self.cosmosdb_client | ||||||||||||||||||||||
or not self.database_client | ||||||||||||||||||||||
or not self.container_client | ||||||||||||||||||||||
): | ||||||||||||||||||||||
return False, "CosmosDB client not initialized correctly" | ||||||||||||||||||||||
return False, "client not initialized" | ||||||||||||||||||||||
Comment on lines
53
to
+58
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The error message 'client not initialized' is less descriptive than the previous 'CosmosDB client not initialized correctly'; consider restoring clarity about which client failed initialization. Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||||||||||||||
try: | ||||||||||||||||||||||
await self.database_client.read() | ||||||||||||||||||||||
except Exception: | ||||||||||||||||||||||
|
@@ -73,7 +79,13 @@ async def create_conversation(self, user_id, title=""): | |||||||||||||||||||||
"updatedAt": datetime.utcnow().isoformat(), | ||||||||||||||||||||||
"userId": user_id, | ||||||||||||||||||||||
"title": title, | ||||||||||||||||||||||
# Metadata field for storing additional information about the conversation. | ||||||||||||||||||||||
# This field is currently an empty dictionary but can be extended in the future. | ||||||||||||||||||||||
# Metadata field for storing additional information about the conversation. | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nitpick] There is a duplicate comment block for the metadata field. Removing the redundant comment would improve clarity and reduce clutter. Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||||||||||||||
# This field is currently an empty dictionary but can be extended in the future. | ||||||||||||||||||||||
"metadata": {} | ||||||||||||||||||||||
} | ||||||||||||||||||||||
validate_metadata(conversation["metadata"]) | ||||||||||||||||||||||
# TODO: add some error handling based on the output of the upsert_item call | ||||||||||||||||||||||
resp = await self.container_client.upsert_item(conversation) | ||||||||||||||||||||||
if resp: | ||||||||||||||||||||||
|
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.
Avoid bare except clauses here as well; handle only expected exceptions or at minimum log unexpected errors to aid troubleshooting.
Copilot uses AI. Check for mistakes.