fix: Adding Python shutdown check in _close to fix sys.meta_path is None error#730
fix: Adding Python shutdown check in _close to fix sys.meta_path is None error#730Bidek56 wants to merge 2 commits intodatabricks:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where closing a Databricks SQL connection during Python shutdown would raise an error due to sys.meta_path being None. The fix adds a shutdown detection mechanism and suppresses error logging when Python is shutting down.
Changes:
- Added Python shutdown detection using
sys.meta_path is Nonecheck - Wrapped close operations in try-except blocks to prevent errors during shutdown
- Suppressed error logging when shutdown is detected to avoid spurious error messages
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Check if Python is shutting down | ||
| shutting_down = sys.meta_path is None | ||
|
|
There was a problem hiding this comment.
The shutdown check variable is computed once at the beginning of _close, but Python shutdown state could change during the execution of this method. Consider checking sys.meta_path is None directly in each exception handler for more accurate shutdown detection.
| try: | ||
| cursor.close() | ||
| except Exception: | ||
| if not shutting_down: | ||
| logger.debug("Error closing cursor during connection close", exc_info=True) |
There was a problem hiding this comment.
Catching bare Exception is too broad and could mask unexpected errors during normal operation. Consider catching specific exceptions or at minimum using except Exception as e: to log the exception details when not shutting down.
| try: | ||
| TelemetryClientFactory.close(host_url=self.session.host) | ||
| except Exception: | ||
| if not shutting_down: | ||
| logger.debug("Error closing telemetry client", exc_info=True) |
There was a problem hiding this comment.
Catching bare Exception is too broad and could mask unexpected errors during normal operation. Consider catching specific exceptions or at minimum using except Exception as e: to log the exception details when not shutting down.
| try: | ||
| self.http_client.close() | ||
| except Exception: | ||
| if not shutting_down: | ||
| logger.debug("Error closing HTTP client", exc_info=True) |
There was a problem hiding this comment.
Catching bare Exception is too broad and could mask unexpected errors during normal operation. Consider catching specific exceptions or at minimum using except Exception as e: to log the exception details when not shutting down.
|
@msrathore-db Please review this PR, it resolves the following error that we are seeing. Thx |
When are you observing this error. Can you provide repro steps? |
I am observing this issue when setting: |
What type of PR is this?
Description
This PR adds Python shutdown check in _close function to avoid this error:
ERROR:Attempt to close session raised a local exception: sys.meta_path is None, Python is likely shutting downHow is this tested?
Related Tickets & Documents