Skip to content

Conversation

@p-
Copy link
Contributor

@p- p- commented Oct 24, 2025

No description provided.

Copilot AI review requested due to automatic review settings October 24, 2025 06:15
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes unnecessary pass statements from the codebase. These statements serve no functional purpose as they appear after other statements in blocks that already contain executable code.

  • Removes redundant pass statements from exception handlers and try blocks
  • Cleans up code in mcp_utils.py, dictionary_file.py, logbook.py, and main.py

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
mcp_utils.py Removes two pass statements from exception handling blocks that already contain other statements
mcp_servers/memcache/memcache_backend/dictionary_file.py Removes pass statement from try block after file operations
mcp_servers/logbook/logbook.py Removes pass statement from try block after file operations
main.py Removes pass statement from except block that already logs an error

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines 229 to 231
logging.error(f"RuntimeError in mcp session task: {e}")
except asyncio.CancelledError as e:
logging.error(f"Timeout on main session task: {e}")
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

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

The asyncio.CancelledError exception is being caught and only logged, which prevents proper task cancellation. CancelledError should typically be re-raised to allow graceful cancellation of the async task. Consider adding raise after the logging statement, or remove this except block entirely to let the exception propagate.

See below for a potential fix:

                    logging.error(f"Timeout on cleanup for mcp server: {server._name}")
                    raise
                finally:
                    mcp_servers.remove(s)
        except RuntimeError as e:
            logging.error(f"RuntimeError in mcp session task: {e}")
        except asyncio.CancelledError as e:
            logging.error(f"Timeout on main session task: {e}")
            raise

Copilot uses AI. Check for mistakes.
@m-y-mo
Copy link
Contributor

m-y-mo commented Oct 24, 2025

@p- Mind leaving the change from main.py out in this PR? I made some big changes to main.py in #33 and would rather not have to rebase. I can incorporate the change you made in main.py in my PR. Thanks.

@p-
Copy link
Contributor Author

p- commented Oct 24, 2025

@m-y-mo Yeah sure 👍 Just incorporate it.

@p- p- merged commit 6443747 into main Oct 24, 2025
5 checks passed
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.

4 participants