Skip to content

Commit

Permalink
fix: appmap breaks vscode python extension starting a REPL
Browse files Browse the repository at this point in the history
  • Loading branch information
zermelo-wisen committed Jun 20, 2024
1 parent 1d5a072 commit c179b86
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 2 deletions.
29 changes: 28 additions & 1 deletion _appmap/env.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ def __init__(self, env=None, cwd=None):
else:
self._env.pop(k, None)

self.log_file_creation_failed = False
self._configure_logging()
enabled = self._env.get("_APPMAP", "false")
self._enabled = enabled is None or enabled.lower() != "false"
Expand Down Expand Up @@ -133,6 +134,21 @@ def display_params(self):
def getLogger(self, name) -> trace_logger.TraceLogger:
return cast(trace_logger.TraceLogger, logging.getLogger(name))

def determine_log_file(self):
log_file = "appmap.log"

# Try creating the log file in the current directory
try:
with open(log_file, 'a', encoding='UTF8'):
pass
except IOError:
# The circumstances in which creation is going to fail
# are also those in which the user doesn't care whether
# there's a log file (e.g. when starting a REPL).
return None
return log_file


def _configure_logging(self):
trace_logger.install()

Expand Down Expand Up @@ -179,13 +195,19 @@ def _configure_logging(self):
log_level = self.get("APPMAP_LOG_LEVEL", "info").upper()
loggers = config_dict["loggers"]
loggers["appmap"]["level"] = loggers["_appmap"]["level"] = log_level

log_file = self.determine_log_file()
# Use NullHandler if log_file is None to avoid complicating the configuration
# with the absence of the "default" handler.
config_dict["handlers"] = {
"default": {
"class": "logging.handlers.RotatingFileHandler",
"formatter": "default",
"filename": "appmap.log",
"filename": log_file,
"maxBytes": 50 * 1024 * 1024,
"backupCount": 1,
} if log_file is not None else {
"class": "logging.NullHandler"
},
"stderr": {
"class": "logging.StreamHandler",
Expand All @@ -194,6 +216,7 @@ def _configure_logging(self):
"stream": "ext://sys.stderr",
},
}
self.log_file_creation_failed = log_file is None

if log_config is not None:
name, level = log_config.split("=", 2)
Expand All @@ -213,3 +236,7 @@ def initialize(**kwargs):
Env.reset(**kwargs)
logger = logging.getLogger(__name__)
logger.info("appmap enabled: %s", Env.current.enabled)
if Env.current.log_file_creation_failed:
# Writing to stderr makes the REPL fail in vscode-python.
# https://github.com/microsoft/vscode-python/blob/c71c85ebf3749d5fac76899feefb21ee321a4b5b/src/client/common/process/rawProcessApis.ts#L268-L269
logger.info("appmap.log cannot be created")
1 change: 1 addition & 0 deletions ci/readonly-mount-appmap.log
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# For a test in smoketest
1 change: 1 addition & 0 deletions ci/run_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ docker run -q -i${t} --rm\
-v $PWD/dist:/dist -v $PWD/_appmap/test/data/unittest:/_appmap/test/data/unittest\
-v $PWD/ci:/ci\
-w /tmp\
-v $PWD/ci/readonly-mount-appmap.log:/tmp/appmap.log:ro\
python:3.11 bash -ce "${@:-/ci/smoketest.sh; /ci/test_pipenv.sh; /ci/test_poetry.sh}"
20 changes: 19 additions & 1 deletion ci/smoketest.sh
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,22 @@ EOF
fi
}

test_log_file_not_writable()
{
cat <<EOF > test_log_file_not_writable.py
import appmap
EOF

python test_log_file_not_writable.py

if [[ $? -eq 0 ]]; then
echo 'Script executed successfully'
else
echo 'Script execution failed'
exit 1
fi
}

set -ex
pip -q install -U pip pytest "flask>=2,<3" python-decouple
pip -q install /dist/appmap-*-py3-none-any.whl
Expand Down Expand Up @@ -48,4 +64,6 @@ else
echo 'No appmap generated?'
find $PWD
exit 1
fi
fi

test_log_file_not_writable

0 comments on commit c179b86

Please sign in to comment.