Skip to content

Clarify that attach --duration keeps tracking in the background#909

Merged
godlygeek merged 1 commit into
bloomberg:mainfrom
swaroski:fix/attach-duration-message
May 10, 2026
Merged

Clarify that attach --duration keeps tracking in the background#909
godlygeek merged 1 commit into
bloomberg:mainfrom
swaroski:fix/attach-duration-message

Conversation

@swaroski
Copy link
Copy Markdown
Contributor

Summary

Test plan

  • memray attach -o out.bin --duration 10 <pid> prints the new notice and exits.
  • memray detach <pid> still stops tracking before duration elapses.
  • Existing tests/unit/test_attach.py passes.

Closes #831

Copy link
Copy Markdown
Contributor

@godlygeek godlygeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks basically good to me. I have 2 small changes I'd like to request.

However, on top of those changes, this branch should only have the one commit on it (it shouldn't have the change from #907 in it) and it needs a Signed-off-by line with your real name in the commit message.

If you amend your commit message to have the right signoff, with:

git commit --amend -C HEAD -m "Signed-off-by: Suyog Bhise <bhise.suyog@gmail.com>"
git push --force-with-lease

then I can drop the other commit and make the other small changes and land this.

Comment thread docs/attach.rst Outdated
Comment thread src/memray/commands/attach.py Outdated
@godlygeek
Copy link
Copy Markdown
Contributor

This one still needs the Signed-off-by: line updated to your real name, @swaroski

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 7, 2026

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 92.37%. Comparing base (796647f) to head (fbc6d21).

Files with missing lines Patch % Lines
src/memray/commands/attach.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #909      +/-   ##
==========================================
- Coverage   92.44%   92.37%   -0.07%     
==========================================
  Files          99       99              
  Lines       11786    11788       +2     
  Branches      428      429       +1     
==========================================
- Hits        10895    10889       -6     
- Misses        891      899       +8     
Flag Coverage Δ
cpp 92.37% <50.00%> (-0.07%) ⬇️
python_and_cython 92.37% <50.00%> (-0.07%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

memray attach -o ... --duration N
  exits as soon as the tracker is injected; the target process keeps recording for N seconds. This is documented but has
   surprised multiple users (bloomberg#701, bloomberg#831).

Print a notice on exit pointing at memray detach for early stop, and call
   out the background behavior explicitly in the attach docs.

Closes bloomberg#831

Signed-off-by: Suyog Bhise <bhise.suyog@gmail.com>
@swaroski swaroski force-pushed the fix/attach-duration-message branch from dd455cd to fbc6d21 Compare May 8, 2026 22:11
Copy link
Copy Markdown
Contributor

@godlygeek godlygeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @swaroski!

@godlygeek godlygeek merged commit 1543e6f into bloomberg:main May 10, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

memray attach with --duration exits immediately instead of tracking for the given time

3 participants