Closed catherinemeadows closed 4 years ago
In response to PR creation
Your results will arrive shortly
Information about docstrings. Just a short comment about the overall functionality of your program and each function.
Not sure what to do about the two linter errors about unused arguments for the handler. That function just listens/catches SIGINT (CTRL -C) to shut down the NFs and the script.
For your handler
function I would follow this ignore pylint error.
For
examples/config.py:120:0: C0301: Line too long (108/100) (line-too-long)
you can always write a \
to split lines (even strings) onto multiple lines.
For your stripping issue, I don't know what the split is doing, but there might be a way to optimize it
Don't fail test
Your results will arrive shortly
Now, a user can specify a directory name in command line (otherwise, default dir name is just the timestamp) so a directory is made, containing a .txt log file for each launched NF (name of log files follow format "log-NF name-instance ID", ex: log-speed_tester-1.txt). Output from NFs now continuously update their corresponding .txt files. User can track output of a particular NF with tail -f logfile
.
To run (within examples folder): python config.py <config file name> [desired dir name]
Screenshot below shows the output of running python config.py example_chain.json
and then in a separate pane, within the timestamp directory that was created, running tail -f log-simple_forward-2.txt
Great addition, will be super helpful for debugging purposes!
@EthanBaron14 when I switched the processes to log the output to files instead of PIPE, it created an error because the error messages couldn't be printed anymore. I fixed this, but do you mind testing again? I wasn't getting a few of the errors that you mentioned in the first place, so I just want to be sure. Specifically, I don't see special characters in the log files and it was exiting fine on Ctrl + C for me, but I can look more into that if you're still getting those errors.
Those fixes have been pushed, but I'll also add functionality for specifying a directory that already exists and some more print statements for when the NFs are starting, etc. Thanks for your review!
I really like what you're doing here. Just a few things I've noticed...
The logging seems to be a little wonky. Not sure what might be causing it, but the log files are changing some characters around. This is an excerpt from the simple_forward log that got generated when running your default configuration:
No IP4 header found �[2J�[1;1HPACKETS ----- Port : 0 Size : 14 N° : 388000000
Both instances of the speed_tester NF have similar issues with logging.
I would personally maybe have one or two messages on the stdout end like "Creating directory", "Starting NFs", "NFs successfully started", or "Shutting down NFs" but that's just personal preference.
In a perfect situation, though, this script runs perfectly. Really, really nice work.
@EthanBaron14 �[2J�[1;1HPACKETS
these are terminal formatters that you're seeing in the logfile. If you run cat logfile
instead, it will print out normally.
I really like what you're doing here. Just a few things I've noticed... The logging seems to be a little wonky. Not sure what might be causing it, but the log files are changing some characters around. This is an excerpt from the simple_forward log that got generated when running your default configuration:
No IP4 header found �[2J�[1;1HPACKETS ----- Port : 0 Size : 14 N° : 388000000
Both instances of the speed_tester NF have similar issues with logging. I would personally maybe have one or two messages on the stdout end like "Creating directory", "Starting NFs", "NFs successfully started", or "Shutting down NFs" but that's just personal preference. In a perfect situation, though, this script runs perfectly. Really, really nice work.
@EthanBaron14
�[2J�[1;1HPACKETS
these are terminal formatters that you're seeing in the logfile. If you runcat logfile
instead, it will print out normally.
Looks like you're right. When I try to view it in any text editor, though, it still has those formatters. Makes it so the only way to view the log is with cat
, right?
Well you can just avoid the formatters when you read it, or we could write something to dynamically remove the formatters before the logfile?
@EthanBaron14 added a try/except block for loading the JSON file for the case that the JSON config has invalid syntax
@onvm quick testing
@onvm quick testing
Your results will arrive shortly
@onvm quick testing
Error: Failed to parse nimbnode23 stats
@onvm again pls
@onvm again pls
Your results will arrive shortly
@catherinemeadows on line 190, can you change the print to print("Starting %s %s" % (nf, cmd), flush=True). Sorry but I need this to make the python server be able to properly close the nf chain.
This looks good to go from my end but I'll let @WilliamMaa make the call since you were assigned
@catherinemeadows and @WilliamMaa will discuss the NF shutdown issue today. @catherinemeadows will post example JSON file
example JSON config file (this is the one that will be included in the /examples folder:
{
"globals": [
{
"TTL": 10
},
{
"directory": "output_files"
}
],
"simple_forward": [
{
"parameters": "2 -d 1"
}
],
"speed_tester": [
{
"parameters": "1 -d 2 -c 16000"
},
{
"parameters": "3 -d 3 -c 16000"
}
]
}
Are there any other global parameters and if so are they defined in the Readme?
Could you easily add a directory-prefix
option, which would let you give a dir name which would then be appended with the date/time? This would be helpful for organizing different experiment runs.
Those are the only two globals I have now. I can add a directory-prefix option. I wrote documentation for the globals and how to run the script here: https://github.com/catherinemeadows/openNetVM/blob/configScript/docs/NF_Dev.md
@WilliamMaa that was just a git mess up on my end. I'll get rid of those changes
Related to issue #184. Created a python script that parses a JSON config file to start up a chain of NFs.
Summary:
This feature makes it easier for the user to launch a chain of NFs by running a python script that parses a JSON Config file containing the desired NFs and parameters. To run the script (within the /examples folder):
python config.py [JSON Config file name]
. Normal shutdown of NFs occurs on user termination (ctrl+c). Shutdown of NFs also occurs if any NF terminates unexpectedly or fails to start up.Documentation for using this feature was added here and here.
Usage:
Merging notes:
TODO before merging :
Test Plan:
To test, I ran the python script to start up chains of NFs. Screenshots from running the script on the example_chain.json file in /examples:
While running, processes are killed on user shutdown (Ctrl + C):
I also tested the script in the case that all NFs do not start (i.e. one or more NF has some error). The script shuts down all processes in this case and outputs the error from the failed NF(s). For this test case, the same config file was used but with a '-d' flag purposefully omitted from the second set of parameters for the speed_tester.![error_exit](https://user-images.githubusercontent.com/43352540/83727002-5ee66100-a612-11ea-85d2-a1cdf641bdf9.png)