Improve logfire run instrumentation recommendations#1922
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
|
Follow-up pushed to address the CI failure in The exact Rich panel snapshot was sensitive to terminal width, so I replaced it with stable assertions for the important output lines, including the new disclaimer and recommended instrumentation packages. Re-ran locally:
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| ╰────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ | ||
|
|
||
| """) | ||
| err = capsys.readouterr().err |
There was a problem hiding this comment.
no, please keep the table assertion.
| for pkg_name, instrumented_pkg in sorted_recommendations: | ||
| recommended_text.append(f'☐ {instrumented_pkg} (need to install {pkg_name})\n', style='grey50') | ||
| if sorted_recommendations: | ||
| recommended_text.append('Only install instrumentation for packages your app actually uses.\n', style='grey50') |
There was a problem hiding this comment.
this isn't really helping. the point is to note the special cases like requests
|
Thanks, I addressed both review comments in follow-ups:
Verified locally:
CI update: |
|
Please open a fresh PR once you have changes again. |
2 participants
Fixes #1296
Summary
logfire runinstrumentation recommendations.Tests
uv run pytest tests/test_cli.py::test_get_recommendation_texts tests/test_cli.py::test_recommended_packages_with_dependencies -quv run ruff check logfire/_internal/cli/run.py tests/test_cli.pygit diff --checkNotes
uv run pytest tests/test_cli.py::test_get_recommendation_texts tests/test_cli.py::test_parse_run_script_with_summary tests/test_cli.py::test_inspect -qpassed the first two tests, buttest_inspectfailed locally on Windows due to existing console/snapshot rendering differences for box-drawing characters.