search

thiezn / iperf3-python

Python wrapper around iperf3
https://iperf3-python.readthedocs.org/
MIT License
110 stars 51 forks source link

"bulksize" parameter #9

Closed cvicente closed 6 years ago

cvicente commented 7 years ago

I wondered what "bulksize" was referring to. A quick glance at the iperf3 code seems to indicate that it actually corresponds to the "-l, --len" parameter in the CLI:

https://github.com/esnet/iperf/blob/a5f5765c8c9f7d3b8b7818addd8ed81a8adee9f5/src/iperf.h#L112

Perhaps "bulksize" should be renamed?

fciaccia commented 7 years ago

As you pointed out the API calls it bulksize and even if I agree that the name is not fully intuitive I think that, being this package a wrapper for libiperf, we should stick to their nomenclature.

cvicente commented 7 years ago

Notice that the API uses "blksize", not "bulksize" (no "u"). That was my point.

On Wed, Sep 28, 2016 at 8:23 AM, Francesco Ciaccia <notifications@github.com

wrote:

As you pointed out the API calls it bulksize and even if I agree that the name is not fully intuitive I think being this package a wrapper for libiperf we should stick to their nomenclature.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/thiezn/iperf3-python/issues/9#issuecomment-250151417, or mute the thread https://github.com/notifications/unsubscribe-auth/AAvcQHO3XorYEESM_TnGzp5iX_V5zA15ks5qulwkgaJpZM4KH1Ib .

fciaccia commented 7 years ago

Sorry, thought you wanted to change it to something like lenght as the CLI option. In that case I agree with you.

thiezn commented 7 years ago

you're right @cvicente, I'll add blksize to be consistent with the libiperf API in the next release. i'll keep bulksize for now as well for backwards compatibility.

A bit swamped at the moment but hope to get another release out in the weekend

thiezn commented 6 years ago

better late than never I'd say...?! Release v0.1.8 (https://github.com/thiezn/iperf3-python/commit/2a6b7f1e24eeda49ba427f6029d7260f27e32b69) adds the blksize argument. I've kept bulksize as an available option to ensure backwards compatibility.