Skip to content
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

fix: appmap breaks vscode python extension starting a REPL #340

Merged
merged 1 commit into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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:
apotterri marked this conversation as resolved.
Show resolved Hide resolved
# 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": {
apotterri marked this conversation as resolved.
Show resolved Hide resolved
"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")
apotterri marked this conversation as resolved.
Show resolved Hide resolved
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
Loading