Persist dojo flask IPython history across container restarts
#730
Conversation
Co-authored-by: zardus <[email protected]>
Co-authored-by: zardus <[email protected]>
Copilot
AI
changed the title
dojo flask historydojo flask IPython history across container restarts
test/test_flask_history.py
Outdated
| @@ -0,0 +1,44 @@ | |||
| import pytest | |||
There was a problem hiding this comment.
These tests are really pointless. You're checking that the ctfd container exists? Why?
If you want a test, make it a functionality test. Check that you can run dojo flask, enter a command, and have a resulting data/ctfd-ipython/profile_default/history.sqlite file created.
There was a problem hiding this comment.
You're absolutely right - those tests were pointless. I've replaced them with a proper functional test that actually runs dojo flask, enters commands, and verifies that /data/ctfd-ipython/profile_default/history.sqlite is created. The test also verifies persistence across multiple flask shell sessions. Fixed in 4f122ce.
Co-authored-by: zardus <[email protected]>
test/test_flask_history.py
Outdated
| from utils import dojo_run | ||
|
|
||
|
|
||
| def is_dojo_environment_available(): |
There was a problem hiding this comment.
this is handled elsewhere in the test harness. Remove it.
There was a problem hiding this comment.
Removed the environment availability check function. Fixed in ac2bcd4.
test/test_flask_history.py
Outdated
| """ | ||
|
|
||
| # Check if the dojo environment is available | ||
| if not is_dojo_environment_available(): |
There was a problem hiding this comment.
Removed the environment check. Fixed in ac2bcd4.
test/test_flask_history.py
Outdated
| if os.path.exists(history_file): | ||
| os.remove(history_file) | ||
|
|
||
| # Ensure the directory exists (should be created by dojo-init) |
There was a problem hiding this comment.
remove this. Assume docker and dojo-init does its job.
There was a problem hiding this comment.
Removed the directory creation - letting dojo-init handle it. Fixed in ac2bcd4.
test/test_flask_history.py
Outdated
| # Verify that the history file was created | ||
| assert os.path.exists(history_file), ( | ||
| f"IPython history file should be created at {history_file} after running flask commands. " | ||
| f"This indicates the volume mount /data/ctfd-ipython:/root/.ipython is working correctly." |
There was a problem hiding this comment.
This text is ambiguous. Does the assertion being thrown indicate that the volume mount is working correctly? Obv not.
There was a problem hiding this comment.
Simplified the assertion message to be clearer and less ambiguous. Fixed in ac2bcd4.
test/test_flask_history.py
Outdated
| f"This confirms history persistence across flask shell sessions." | ||
| ) | ||
|
|
||
| print(f"✓ IPython history persistence test passed") |
There was a problem hiding this comment.
remove these prints. pytest prints the checkmark
There was a problem hiding this comment.
Removed all print statements - letting pytest handle output. Fixed in ac2bcd4.
test/test_flask_history.py
Outdated
|
|
||
| except subprocess.TimeoutExpired: | ||
| pytest.fail( | ||
| "Flask shell command timed out. This may indicate:\n" |
There was a problem hiding this comment.
don't catch these errors; just let pytest catch them and fail accordingly
There was a problem hiding this comment.
Removed manual exception handling - letting pytest catch and fail appropriately. Fixed in ac2bcd4.
…d error handling Co-authored-by: zardus <[email protected]>
|
why fail? |
…ally fails Co-authored-by: zardus <[email protected]>
You're right - the flask command should exit cleanly with code 0 when given valid input. Removed the |
|
what??? no, i mean, why are the tests failing? |
…tive shells exit with non-zero codes Co-authored-by: zardus <[email protected]>
The tests were failing because the |
The
dojo flaskcommand opens an IPython shell in the ctfd container, but the history was being saved in a container-local file and lost ondojo bounce. This adds persistence by volume-mounting the IPython configuration directory.Changes Made
/data/ctfd-ipython:/root/.ipythonto the ctfd servicemkdir -p /data/ctfd-ipythonHow it works
dojo flaskruns, it executesdocker exec ctfd flask shell~/.ipython//root/.ipython/data/ctfd-ipythonon the hostdojo bounceBenefits
The implementation follows existing patterns in the codebase (mounting
/data/*directories for persistence) and makes minimal surgical changes to solve the issue.Fixes #729.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.