-
Notifications
You must be signed in to change notification settings - Fork 28
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
lf_interop_throughput.py: LAN-3628 - Verify CX 'run' state prior to starting tests and improved flake8 compliance #76
base: master
Are you sure you want to change the base?
Conversation
Updated all flake8 complaints to make script compliant. A few noqa in line comments inserted that can be improved later. Verified still functional with: ./lf_interop_throughput.py --mgr 192.168.91.50 --mgr_port 8080 --ssid Quincefruit --passwd lanforge --security wpa2 --upstream_port 1.050.01 --traffic_type lf_udp --download 100000000 --test_duration 1m --packet_size 1500 --incremental_capacity 1 --precleanup --postcleanup Signed-off-by: Cameron LaPlante <Cameron.laplante@candelatech.com>
a90b8ac
to
3bbf3ce
Compare
Moved arg parsing to parse_args function, and added validation for required arguments (validate_args). Verified with: ./lf_interop_throughput.py --mgr 192.168.91.50 --mgr_port 8080 --ssid Quincefruit --passwd lanforge --security wpa2 --upstream_port 1.050.01 --traffic_type lf_udp --download 100000000 --test_duration 1m --packet_size 1500 --incremental_capacity 1 --precleanup --postcleanup Signed-off-by: Cameron LaPlante <Cameron.laplante@candelatech.com>
Added get_cx_states to validate 'Run' state before beginning specified test duration. Verified with: ./lf_interop_throughput.py --mgr 192.168.91.50 --mgr_port 8080 --ssid Quincefruit --passwd lanforge --security wpa2 --upstream_port 1.050.01 --traffic_type lf_udp --download 100000000 --test_duration 1m --packet_size 1500 --incremental_capacity 1 --precleanup --postcleanup Signed-off-by: Cameron LaPlante <Cameron.laplante@candelatech.com>
Adding some headers to functions, included 'r' in top header, general tidying. Verified with: ./lf_interop_throughput.py --mgr 192.168.91.50 --mgr_port 8080 --ssid Quincefruit --passwd lanforge --security wpa2 --upstream_port 1.050.01 --traffic_type lf_udp --download 100000000 --test_duration 1m --packet_size 1500 --incremental_capacity 1 --precleanup --postcleanup Signed-off-by: Cameron LaPlante <Cameron.laplante@candelatech.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather we use the required=True
kwarg in the add_argument() function for each of the default arguments, instead of checking for them in argument validation.
Can you also check w/ CT-IN to verify adding this check doesn't break any assumptions their automation may make? I worry that introducing these checks may break existing WebGUI assumptions
val Added max_retries to cx states loop to avoid infinite checking if error occurs.i Removed validate args function and used kwarg required=True instead. Verified with: ./lf_interop_throughput.py --mgr 192.168.91.50 --mgr_port 8080 --ssid Quincefruit --passwd lanforge --security wpa2 --upstream_port 1.050.01 --traffic_type lf_udp --download 100000000 --test_duration 1m --packet_size 1500 --incremental_capacity 1 --precleanup --postcleanup Signed-off-by: Cameron LaPlante <Cameron.laplante@candelatech.com>
d9c60d7
to
da14561
Compare
Corrected myriad of flake8 compliance issues.
Added cx state validation before running test duration to avoid logging zero throughput on test.
Improved clarity of required arguments by moving arg parsing and adding validation to exit gracefully.
Verified with:
./lf_interop_throughput.py --mgr 192.168.91.50 --mgr_port 8080 --ssid Quincefruit --passwd lanforge --security wpa2 --upstream_port 1.050.01 --traffic_type lf_udp --download 100000000 --test_duration 1m --packet_size 1500 --incremental_capacity 1 --precleanup --postcleanup
Signed-off-by: Cameron LaPlante Cameron.laplante@candelatech.com