Feature pipe captures #280
Conversation
|
Dor, I have a lot of code replication in the PipeRingCapture. Any way I can avoid that? I overrode |
| import tempfile | ||
|
|
||
|
|
||
| class DisplayFilterNotAllowedException(Exception): |
There was a problem hiding this comment.
Due to how RingCaptures work, Tshark will not allow a display filter in this class.
|
About the code duplication: |
|
@KimiNewt Looking good now? |
| :param override_prefs: A dictionary of tshark preferences to override, {PREFERENCE_NAME: PREFERENCE_VALUE, ...}. | ||
| :param disable_protocol: Tells tshark to remove a dissector for a specifc protocol. | ||
| """ | ||
| if display_filter is not None: |
There was a problem hiding this comment.
It's possible simply not to have it as a parameter in the init and just to write it in the doc. This is a bit more explicit so I'm not 100% about it.
src/pyshark/capture/pipe_capture.py
Outdated
|
|
||
|
|
||
| class PipeCapture(Capture): | ||
| class PipeCapture(LiveCapture): |
There was a problem hiding this comment.
Does this change not break anything about the previous usage of PipeCapture?
LiveCapture receives an interface as a string, yet here pipe is (unless I'm mistaken) a pipe-like object.
src/pyshark/capture/pipe_capture.py
Outdated
|
|
||
| def close(self): | ||
| # Close pipe | ||
| self._pipe.close() | ||
| # self._pipe.close() # Don't close the pipe. This should be the job of whatever is piping into it. |
There was a problem hiding this comment.
That's fine, but remove this line entirely and add this to the class doc in CAPS or otherwise in an obvious manner
src/pyshark/capture/pipe_capture.py
Outdated
| @@ -40,8 +40,10 @@ def get_parameters(self, packet_count=None): | |||
| return params[:-1] | |||
|
|
|||
| def close(self): | |||
| # Close pipe | |||
| # self._pipe.close() # Don't close the pipe. This should be the job of whatever is piping into it. | |||
| """ | |||
There was a problem hiding this comment.
You can actually just remove this entire method now, the docs in the classdoc should suffice
| @@ -38,10 +38,10 @@ def __init__(self, pipe, ring_file_size=1024, num_ring_files=2, ring_file_name=N | |||
| :param override_prefs: A dictionary of tshark preferences to override, {PREFERENCE_NAME: PREFERENCE_VALUE, ...}. | |||
| :param disable_protocol: Tells tshark to remove a dissector for a specifc protocol. | |||
| """ | |||
| if display_filter is not None: | |||
| if "display_filter" in kwargs.keys(): | |||
There was a problem hiding this comment.
You can remove this as well as the exception. I don't think it's needed. Again, just write in the doc that display filters are not supported.
If you feel strongly about having this exception, then it's better the previous way. This is bad because it "swallows" any kwargs the user may have inputted (say they misspelt use_json, it'll suddenly not fail but not work either)
There was a problem hiding this comment.
While I agree, I should pass them up to the super, I think it's important to do a hard exception for this since every other capture allows it.
|
Sorry this one took so long, got busy with some other stuff. |
|
I'm using a subset of this PR and it works fine. Could you please merge at least the |
Working on enabling PipeCaptures as well as updating a few other features.
It's Work In Progress, so don't merge yet. I just want to make sure you can put in comments early.