Skip to content

Conversation

@tleonhardt
Copy link
Member

@tleonhardt tleonhardt commented Mar 19, 2019

The load command now supports the -t/--transcript flag for recording a transcript file based on a script file like so:

20:44 $ examples/hello_cmd2.py
(Cmd) load examples/scripts/script.txt -t tran.txt
2 commands and their outputs saved to transcript file 'tran.txt'
(Cmd)

Also updated the documentation for scripts and transcripts accordingly.

This closes #639
This closes #651

TODO

  • Add a unit test
  • Updated the CHANGELOG
  • Consider adding more unit tests
  • Wait for the pyscript fix by @kmvanbrunt to merge first

The load command now supports the -r/--record_transcript flag for recording a transcript file based on a script file.
@tleonhardt tleonhardt self-assigned this Mar 19, 2019
@tleonhardt tleonhardt requested review from kmvanbrunt and kotfu March 19, 2019 00:50
@codecov
Copy link

codecov bot commented Mar 19, 2019

Codecov Report

Merging #652 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #652      +/-   ##
==========================================
- Coverage   94.48%   94.46%   -0.02%     
==========================================
  Files          11       11              
  Lines        3062     3072      +10     
==========================================
+ Hits         2893     2902       +9     
- Misses        169      170       +1
Impacted Files Coverage Δ
cmd2/cmd2.py 94.62% <100%> (-0.03%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e89bd4f...74d2d60. Read the comment docs.

@tleonhardt tleonhardt added this to the 0.9.12 milestone Mar 19, 2019
Also:
- Removed guard clauses which kmvanbrunt promises will be unecessary with his upcoming change
- Moved transcript path validation inside _generate_transcript()
@teto
Copy link

teto commented Mar 19, 2019

My current code used to run with 0.9.10, I wanted to try this PR and it failed with

Traceback (most recent call last):
 File "/home/teto/mptcpanalyzer/mptcpanalyzer/cli.py", line 1294, in main
   analyzer = MpTcpAnalyzerCmdApp(config, **vars(args))
 File "/home/teto/mptcpanalyzer/mptcpanalyzer/cli.py", line 172, in __init__
   self.shortcuts.update({
 File "/nix/store/sl63h85niy4qbvryjf82q278k45isfab-python3.7-cmd2-0.9.10/lib/python3.7/site-packages/cmd2/cmd2.py", line 570, in shortcuts
   return self.statement_parser.shortcuts
AttributeError: 'MpTcpAnalyzerCmdApp' object has no attribute 'statement_parser'

my current code is


        self.shortcuts.update({
            'lm': 'list_mptcp_connections',
            'lt': 'list_tcp_connections',
            'ls': 'list_subflows',
            'lr': 'list_reinjections'
        })
        super().__init__(completekey='tab', stdin=stdin)

the doc says to pass it as a parameter now (might be cleaner) but I am confused by the self.DEFAULT_SHORTCUTS. is DEFAULT_SHORTCUTS a thing of cmd2 ? or is it just for the example ? otherwise Cmd2.init could add the default shortcuts.
https://cmd2.readthedocs.io/en/latest/settingchanges.html?highlight=shortcut

@tleonhardt
Copy link
Member Author

tleonhardt commented Mar 19, 2019

@teto Well you are the first to encounter some cleanup code that we just merged in tonight. It is a breaking change, but we have been trying to make the hard choices prior to a 1.0 release to cleanup certain things.

You should be able to modify your current code to:

        shortcuts = dict(self.DEFAULT_SHORTCUTS)
        shortcuts.update({
            'lm': 'list_mptcp_connections',
            'lt': 'list_tcp_connections',
            'ls': 'list_subflows',
            'lr': 'list_reinjections'
        })
        super().__init__(completekey='tab', stdin=stdin, shortcuts=shortcuts)

I thought of potentially making cmd2.Cmd.__init__() automatically merge in the default shortcuts, but I didn't want to force shortcuts on anybody - I wanted them to be able to not use certain of the default shortcuts. Now in the new cleaner architecture, once shortcuts get set, they are immutable.

@teto
Copy link

teto commented Mar 19, 2019

It does what I want and I think it works fine but I put the results here just in case you are interested.
My only concern is that history creates the transcripts with -t and this one with -r; maybe flags should be harmonized ?

Here my tests/script_mptcp.txt

load_pcap examples/client_2.pcap
list_mptcp_connections
list_subflows 0
list_subflows 1

testing the command (it doesn't print the command name but maybe it's because I customized something)

load tests/script_mptcp.txt -r toto
>>> tests/script_mptcp.txt -r toto
>>> examples/client_2.pcap
ran cmd ['tshark', '-E', 'header=y', '-r', '/home/teto/mptcpanalyzer/examples/client_2.pcap', '-E', 'separator=|', '-o', 'gui.column.format:"Time","%At","ipsrc","%s","ipdst","%d"', '-o', 'tcp.analyze_sequence_numbers:True', '-o', 'mptcp.analyze_mappings:True', '-o', 'mptcp.relative_sequence_numbers:True', '-o', 'mptcp.intersubflows_retransmission:True', '-o', 'mptcp.analyze_mptcp:True', '-2', '-R', 'mptcp or tcp and not icmp', '-T', 'fields', '-e', 'frame.number', '-e', 'frame.time_relative', '-e', 'frame.time_epoch', '-e', '_ws.col.ipsrc', '-e', '_ws.col.ipdst', '-e', 'ip.src_host', '-e', 'ip.dst_host', '-e', 'tcp.stream', '-e', 'tcp.srcport', '-e', 'tcp.dstport', '-e', 'tcp.window_size', '-e', 'tcp.flags', '-e', 'tcp.option_kind', '-e', 'tcp.seq', '-e', 'tcp.len', '-e', 'tcp.ack', '-e', 'tcp.options.timestamp.tsval', '-e', 'tcp.options.timestamp.tsecr', '-e', 'mptcp.expected_token', '-e', 'mptcp.stream', '-e', 'tcp.options.mptcp.sendkey', '-e', 'tcp.options.mptcp.recvkey', '-e', 'tcp.options.mptcp.recvtok', '-e', 'tcp.options.mptcp.datafin.flag', '-e', 'tcp.options.mptcp.subtype', '-e', 'tcp.options.mptcp.rawdataseqno', '-e', 'tcp.options.mptcp.rawdataack', '-e', 'tcp.options.mptcp.subflowseqno', '-e', 'tcp.options.mptcp.datalvllen', '-e', 'tcp.options.mptcp.addrid', '-e', 'mptcp.rawdsn64', '-e', 'mptcp.ack', '-e', 'mptcp.dsn', '-e', 'mptcp.related_mapping', '-e', 'mptcp.reinjection_of', '-e', 'mptcp.reinjected_in']
stderr= 
** (process:20295): WARNING **: 15:35:57.351: Obsolete preference "gui.webbrowser" at line 103 of
/home/teto/.config/wireshark/preferences (save preferences to remove this warning)

>>> 
>>> 0
>>> 1
4 commands and their outputs saved to transcript file 'toto'

generated this output

�[34mReady>�[39m�[49mload_pcap examples/client_2.pcap
Loading examples\/client_2.pcap
client_2.pcap> list_mptcp_connections
4 mptcp connection(s)
mptcp.stream 0 has 4 subflow(s) (client\/server): 
	tcp.stream 0: 10.0.0.1:59482  -> 10.0.0.2:05201  (mptcpdest: ConnectionRoles.Server)
	tcp.stream 1: 11.0.0.1:60453  -> 11.0.0.2:05201  (mptcpdest: ConnectionRoles.Server)
	tcp.stream 2: 10.0.0.1:49807  -> 11.0.0.2:05201  (mptcpdest: ConnectionRoles.Server)
	tcp.stream 6: 11.0.0.1:55233  -> 10.0.0.2:05201  (mptcpdest: ConnectionRoles.Server)

mptcp.stream 1 has 4 subflow(s) (client\/server): 
	tcp.stream 3: 10.0.0.1:59484  -> 10.0.0.2:05201  (mptcpdest: ConnectionRoles.Server)
	tcp.stream 4: 10.0.0.1:35031  -> 11.0.0.2:05201  (mptcpdest: ConnectionRoles.Server)
	tcp.stream 5: 11.0.0.1:34945  -> 11.0.0.2:05201  (mptcpdest: ConnectionRoles.Server)
	tcp.stream 7: 11.0.0.1:40191  -> 10.0.0.2:05201  (mptcpdest: ConnectionRoles.Server)

mptcp.stream 2 has 4 subflow(s) (client\/server): 
	tcp.stream 8: 10.0.0.1:59486  -> 10.0.0.2:05201  (mptcpdest: ConnectionRoles.Server)
	tcp.stream 9: 10.0.0.1:57565  -> 11.0.0.2:05201  (mptcpdest: ConnectionRoles.Server)
	tcp.stream 10: 11.0.0.1:36829  -> 10.0.0.2:05201  (mptcpdest: ConnectionRoles.Server)
	tcp.stream 11: 11.0.0.1:40045  -> 11.0.0.2:05201  (mptcpdest: ConnectionRoles.Server)

mptcp.stream 3 has 4 subflow(s) (client\/server): 
	tcp.stream 12: 10.0.0.1:59488  -> 10.0.0.2:05201  (mptcpdest: ConnectionRoles.Server)
	tcp.stream 13: 10.0.0.1:54007  -> 11.0.0.2:05201  (mptcpdest: ConnectionRoles.Server)
	tcp.stream 14: 11.0.0.1:35041  -> 10.0.0.2:05201  (mptcpdest: ConnectionRoles.Server)
	tcp.stream 15: 11.0.0.1:39755  -> 11.0.0.2:05201  (mptcpdest: ConnectionRoles.Server)

client_2.pcap> list_subflows 0
mptcp.stream 0 has 4 subflow(s) (client\/server): 
	tcp.stream 0: 10.0.0.1:59482  -> 10.0.0.2:05201  (mptcpdest: ConnectionRoles.Server)
	tcp.stream 1: 11.0.0.1:60453  -> 11.0.0.2:05201  (mptcpdest: ConnectionRoles.Server)
	tcp.stream 2: 10.0.0.1:49807  -> 11.0.0.2:05201  (mptcpdest: ConnectionRoles.Server)
	tcp.stream 6: 11.0.0.1:55233  -> 10.0.0.2:05201  (mptcpdest: ConnectionRoles.Server)
client_2.pcap> list_subflows 1
mptcp.stream 1 has 4 subflow(s) (client\/server): 
	tcp.stream 3: 10.0.0.1:59484  -> 10.0.0.2:05201  (mptcpdest: ConnectionRoles.Server)
	tcp.stream 4: 10.0.0.1:35031  -> 11.0.0.2:05201  (mptcpdest: ConnectionRoles.Server)
	tcp.stream 5: 11.0.0.1:34945  -> 11.0.0.2:05201  (mptcpdest: ConnectionRoles.Server)
	tcp.stream 7: 11.0.0.1:40191  -> 10.0.0.2:05201  (mptcpdest: ConnectionRoles.Server)

@tleonhardt
Copy link
Member Author

Thanks for the feedback. I will change both commands to use a -t flag for consistency.

@tleonhardt tleonhardt changed the title Added load -r flag for recording a transcript based on a script file Added load -t flag for recording a transcript based on a script file Mar 20, 2019
@tleonhardt tleonhardt merged commit 9f5906a into master Mar 20, 2019
@tleonhardt tleonhardt deleted the load_generate_transcript branch March 20, 2019 00:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Document expected behavior when output of load command is redirected or piped transcript generation/update

4 participants