Skip to content

Commit

Permalink
fix: appmap.Recording is available even when APPMAP=false
Browse files Browse the repository at this point in the history
  • Loading branch information
zermelo-wisen committed May 30, 2024
1 parent fa33b89 commit d7c522e
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 6 deletions.
8 changes: 4 additions & 4 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ dist: jammy
language: python
python:
- "3.12"
- "3.11"
- "3.10"
- "3.9.14"
- "3.8"
# - "3.11"
# - "3.10"
# - "3.9.14"
# - "3.8"

# https://github.com/travis-ci/travis-ci/issues/1147#issuecomment-441393807
if: type != push OR branch = master OR branch =~ /^v\d+\.\d+(\.\d+)?(-\S*)?$/
Expand Down
27 changes: 27 additions & 0 deletions _appmap/recording.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,33 @@ def __exit__(self, exc_type, exc_value, tb):
return False


class NoopRecording:
"""
A noop context manager to export as "Recording" instead of class
Recording when not Env.current.enabled.
"""

def __init__(self, exit_hook=None):
self.exit_hook = exit_hook

def start(self):
pass

def stop(self):
pass

def is_running(self):
return False

def __enter__(self):
pass

def __exit__(self, exc_type, exc_value, tb):
if self.exit_hook is not None:
self.exit_hook(self)
return False


def write_appmap(
appmap, appmap_fname, recorder_type, metadata=None, basedir=Env.current.output_dir
):
Expand Down
11 changes: 10 additions & 1 deletion appmap/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
# execute the imports in a function, the modules all get put into the funtion's
# globals, rather than into appmap's globals.
_enabled = os.environ.get("APPMAP", None)
_recording_exported = False
if _enabled is None or _enabled.upper() == "TRUE":
if _enabled is not None:
# Use setdefault so tests can manage _APPMAP as necessary
Expand All @@ -18,6 +19,7 @@
from _appmap.labels import labels # noqa: F401
from _appmap.noappmap import decorator as noappmap # noqa: F401
from _appmap.recording import Recording # noqa: F401
_recording_exported = True

try:
from . import django # noqa: F401
Expand Down Expand Up @@ -51,4 +53,11 @@ def enabled():
else:
os.environ.pop("_APPMAP", None)
else:
os.environ.setdefault("_APPMAP", "false")
os.environ.setdefault("_APPMAP", "false")

if not _recording_exported:
# Client code that imports appmap.Recording should run correctly
# even when not Env.current.enabled (not APPMAP=true).
# This prevents:
# ImportError: cannot import name 'Recording' from 'appmap'...
from _appmap.recording import NoopRecording as Recording # noqa: F401
24 changes: 23 additions & 1 deletion ci/smoketest.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,24 @@
#!/usr/bin/env bash

test_recording_when_appmap_not_true()
{
cat <<EOF > test_client.py
from appmap import Recording
with Recording():
print("Hello from appmap library client")
EOF

python test_client.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 All @@ -9,6 +28,9 @@ cp -R /_appmap/test/data/unittest/simple ./.
# Before we enable, run a command that tries to load the config
python -m appmap.command.appmap_agent_status

# Ensure that client code using appmap.Recording does not fail when not APPMAP=true
test_recording_when_appmap_not_true

export APPMAP=true

python -m appmap.command.appmap_agent_init |\
Expand All @@ -26,4 +48,4 @@ else
echo 'No appmap generated?'
find $PWD
exit 1
fi
fi

0 comments on commit d7c522e

Please sign in to comment.