print stacktraces during import errors #10906
Conversation
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
cosmicexplorer
force-pushed
the
log-stacktrace-on-import-error
branch
from
5f3e4cc to
8902834
Compare
| if cls._logging_initialized: | ||
| logger.exception(exc) | ||
| else: | ||
| exception_log_entry = cls._format_unhandled_exception_log( |
There was a problem hiding this comment.
What happens if we get rid of the logic for checking and setting cls._logging_initialized and just always use the cls._format_unhandled_exception_log form? If I understand https://docs.python.org/3.6/library/logging.html#logging.Logger.exception correcty, logger.exception and logger.error should both be writing fairly similar text to the Rust logging faculty, once it's been initialized. If we could get away with just modifying this one line to print the additional context we need for import-time errors, without adding any additional logic to think about in this class, I think that would be ideal.
There was a problem hiding this comment.
That works, but why doesn't it print the stacktrace twice when we send the rust logger the stacktrace? What is it doing such that sending it the exception vs a completely different string with the exception + stacktrace produces the same output?
There was a problem hiding this comment.
That works, but why doesn't it print the stacktrace twice when we send the rust logger the stacktrace? What is it doing such that sending it the exception vs a completely different string with the exception + stacktrace produces the same output?
I'm not sure. Does anything change if you call logger.error with the string generated by format_unhandled_exception_log rather than logger.exception?
There was a problem hiding this comment.
No! Which is so weird! I would like to understand this part further, because it's not immediately clear to me, but I wasn't able to easily figure it out by scanning the rust logger implementation, and the right behavior still seems to occur on --no-print-exception-stacktrace.
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
Commit message needs to be updated |
Internal only changes left off from the changelog: * Use cpython types in Rust functions that manipulate python objects (#10942) `PR #10942 <https://github.com/pantsbuild/pants/pull/10942>`_ * update libz-sys version to fix macOS compile error (#10941) `PR #10941 <https://github.com/pantsbuild/pants/pull/10941>`_ * Upgrade to Rust stable 1.47.0. (#10933) `PR #10933 <https://github.com/pantsbuild/pants/pull/10933>`_ * Finish CreateDigest Directory cleanup. (#10935) `PR #10935 <https://github.com/pantsbuild/pants/pull/10935>`_ * Hotfix broken import from merge conflict (#10934) `PR #10934 <https://github.com/pantsbuild/pants/pull/10934>`_ * Revert "Port nailgun client to rust (#10865)" (#10929) `PR #10929 <https://github.com/pantsbuild/pants/pull/10929>`_ * An ExternalTool for downloading the grpc_python_plugin. (#10927) `PR #10927 <https://github.com/pantsbuild/pants/pull/10927>`_ * Port nailgun client to rust (#10865) `PR #10865 <https://github.com/pantsbuild/pants/pull/10865>`_ * print stacktraces during import errors (#10906) `PR #10906 <https://github.com/pantsbuild/pants/pull/10906>`_ * fs.Digest is declared in Rust (#10905) `PR #10905 <https://github.com/pantsbuild/pants/pull/10905>`_ * add requests_ca_bundle to settable_env_vars (#10909) `PR #10909 <https://github.com/pantsbuild/pants/pull/10909>`_ [ci skip-rust]
[ci skip-rust]
Problem
Pants only prints what failed to import upon an
ImportError, not where.