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

Allow using PySnooper as a command line program #145

Closed
wants to merge 3 commits into from
Closed

Allow using PySnooper as a command line program #145

wants to merge 3 commits into from

Conversation

tebeka
Copy link

@tebeka tebeka commented Jul 28, 2019

A POC for running pysnooper as an external command (e.g. pysnooper /path/to/script.py)

The script will create sitecustomize.py script and will add it first in PYTHONPATH.

@cool-RR
Copy link
Owner

cool-RR commented Jul 28, 2019

Interesting. Did you find yourself wanting this functionality in a real-world scenario?

@tebeka
Copy link
Author

tebeka commented Jul 29, 2019

I haven't used PySnooper is real world scenario. However first time I played with PySnooper I was looking for something like this to inspect my code without modifying it.

@tebeka
Copy link
Author

tebeka commented Jul 29, 2019

If the concept is appvroved, I'll work on what's left:

  • tests
  • command line options for most (all) of snoop parameters (output, depth, prefix ...)
    -`

@cool-RR
Copy link
Owner

cool-RR commented Jul 29, 2019

This sounds great, consider it approved.

@alexmojaki if you have an opinion about this, I'll be happy to hear it, and of course feel free to merge it to your fork if you're interested.

@cool-RR
Copy link
Owner

cool-RR commented Jul 29, 2019

@tebeka If you get stuck on anything or want me to help implement anything, let me know!

@alexmojaki
Copy link
Collaborator

I haven't tried it myself but in its current form it looks like it would only trace the created sitecustomize.py, so I'm a bit confused. Does this work?

I think it's best that this branch gets used in a real world scenario first. The benefit of not modifying source code is pretty minimal, and this seems like it would be rarely useful. It's not often that you want to start tracing from the top level, and it may take rather high depth to get where you want, which will create lots of noise. Perhaps it would be better if you could specify the name of one or more functions to trace, to mimic the decorator.

@alexmojaki
Copy link
Collaborator

Maybe a good alternative method to not alter the source would be environment variable activation similar to better-exceptions. There are plenty of situations when a custom run command is not an option, especially if you're already using a command from a different library (e.g. pytest). @tebeka what would you think of that?

@cool-RR
Copy link
Owner

cool-RR commented Jul 30, 2019

@alexmojaki I hope I can take credit for making you more cautious about adding potentially esoteric functionality :)

@tebeka If you get it working right and well-tested, I'll merge it.

@cool-RR cool-RR changed the title command POC Allow using PySnooper as a command line program Jul 30, 2019
@tebeka
Copy link
Author

tebeka commented Aug 1, 2019

@alexmojaki

I haven't tried it myself but in its current form it looks like it would only trace the created sitecustomize.py, so I'm a bit confused. Does this work?

It works since I start the snooper at sitecustomize.py (which is loaded by Python) and do not close it.

The benefit of not modifying source code is pretty minimal,

I disagree :) Most profiling/debugging tools work without requiring the user to modify their code. I would't like to set breakpoint by manually adding breakpoint() at my code.

It's not often that you want to start tracing from the top level, and it may take rather high depth to get where you want

I agree that in the future we might add a way to start tracing only from the user script or many support some command line options to specify which functions to monitor (some kind of filter probably). But let's start with a simple thing and move on from there.

Maybe a good alternative method to not alter the source would be environment variable activation

This will still require the user to change their source code.

There are plenty of situations when a custom run command is not an option, especially if you're already using a command from a different library (e.g. pytest).

Your right in the general case, but I try to solve a specific case and not a general one. In the case of pytest this can be done via a plugin.

@tebeka
Copy link
Author

tebeka commented Aug 1, 2019

Hmm, for some reason I don't see the traced program output anymore (like @alexmojaki said). Probably due to merge with master. Will investigate

@alexmojaki
Copy link
Collaborator

This will still require the user to change their source code.

No it wouldn't. It sounds like you didn't look at what better-exceptions does. It provides a .pth file that automatically activates when you specify an environment variable. Another example which is more like PySnooper is hunter.

@tebeka
Copy link
Author

tebeka commented Aug 1, 2019

@cool-RR Can you take a look now?

@alexmojaki
Copy link
Collaborator

This implementation seems very complicated - sitecustomize, subprocess, environment variables...

Try something like this:

code_obj = compile(source, filename, 'exec')
tracer.target_codes.add(code_obj)
exec(code)

That's roughly how the IPython integration of snoop works: https://github.com/alexmojaki/snoop/blob/master/snoop/ipython.py

@cool-RR
Copy link
Owner

cool-RR commented Aug 3, 2019

@tebeka I reviewed the code and found a couple of problems:

  1. The _should_trace refactoring is elegant, but I think it might hurt performance. It's important that for lines that shouldn't be traced, the trace function will return as quickly as possible. Every function call matters because the trace function could be called millions of times.
  2. I'm a little wary about the fact that your test finds the Python executable and runs it in a separate process. I think that maybe things can go wrong that way. Is there no way to test it using exec or something.

Also, Alex's input above. Also, the tests fail.

If you find this too overwhelming, I'd understand if you'd abandon the PR. I think its value is marginal, so I'd only accept it if it's really well done.

@tebeka
Copy link
Author

tebeka commented Aug 4, 2019

@alexmojaki

This will still require the user to change their source code.

No it wouldn't. It sounds like you didn't look at what better-exceptions does. It provides a .pth file that automatically activates when you specify an environment variable. Another example which is more like PySnooper is hunter.

You're right, my bad - Sorry.

However I dislike things the globally change everything in the Python interpreter. Also the code in the .pth looks pretty ugly to me :)

code_obj = compile(source, filename, 'exec')

Most cases for command line script you'd like to pass some parameters. This becomes difficult in thsis approach.

@tebeka
Copy link
Author

tebeka commented Aug 4, 2019

@cool-RR

The _should_trace refactoring is elegant, but I think it might hurt performance. It's important that for lines that shouldn't be traced, the trace function will return as quickly as possible. Every function call matters because the trace function could be called millions of times.

I understand the concern. However this statement needs to be backed up by profiling :)

I'm a little wary about the fact that your test finds the Python executable and runs it in a separate process. I think that maybe things can go wrong that way. Is there no way to test it using exec or something.

I'm trying to test the actual script. If things go wrong - it's probably a bug.

Also, Alex's input above. Also, the tests fail.
Answered Alex. Will look at test.

If you find this too overwhelming, I'd understand if you'd abandon the PR. I think its value is marginal, so I'd only accept it if it's really well done.
Totally understand. Seems from your & @alexmojaki comment that this PR is not well aligned with the project. I'm closing it for now.

@tebeka tebeka closed this Aug 4, 2019
@alexmojaki
Copy link
Collaborator

Hey @tebeka, thanks for your time. I suggest using PySnooper (or snoop) in a real scenario a few times to understand what using it is like. If you find a situation where you'd really like this feature let us know and we might be convinced to add it. I might even implement it myself.

There's also the possibility of tracing many functions without putting @snoop on each one, so you modify some source but not too much. I've suggested some possibilities here and would be interested in your opinion.

@cool-RR
Copy link
Owner

cool-RR commented Aug 28, 2019

Recently I found myself wanting this feature a few times, so I think it's a pretty good idea now.

If anyone were interested in implementing it according to our notes above, that'd be cool.

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.

None yet

3 participants