search

SIPp / pysipp

SIPp for Humans - launch multiple agents with Python
GNU General Public License v2.0
179 stars 55 forks source link

Feature/support disabling screen_file argument #40

Closed y-luis-rojo closed 6 years ago

y-luis-rojo commented 6 years ago

Allow disabling screen_file argument.

goodboy commented 6 years ago

@y-luis do you mind squashing this history down to a single commit. Lot of noise in that commit chain ;)

y-luis-rojo commented 6 years ago

Done ;)

goodboy commented 6 years ago

@y-luis awesome :+1:

I'm just trying to remember (will look at the code later) exposing this in Agent.__init__() means the public apis in pysipp/__init__.py also have direct exposure right?

I guess what I'm asking is what is the code you are using right now to disable logs (with these changes in)? Also we should probably have a test for this I'm thinking at least to verify the api?

y-luis-rojo commented 6 years ago

This comes from using pysipp with sipp version 3.3 which does not support log files. The way this change can be used is passing the logs argument to scenario creation:

scen = pysipp.scenario(dirpath=temp_dir, enable_screen_file=False)

goodboy commented 6 years ago

@y-luis hmm I wonder if it's sipp version specific maybe we should have some logic to detect that and automatically disable logs - or at the least document this option in the readme.

y-luis-rojo commented 6 years ago

I agree @tgoodlet. I can document that in the README, but I do not know which is the minimum version supporting log files. Will have to research it.

y-luis-rojo commented 6 years ago

Seems -screen_file argument was added at version 3.5.0: https://sourceforge.net/p/sipp/mailman/message/34041962/

goodboy commented 6 years ago

@y-luis hmm yeah maybe worth documenting as well. sipp versions was not something that was scrutinized too much with this project - it was built on whatever the latest version was at the time.

y-luis-rojo commented 6 years ago

@tgoodlet, feature fixed to only disable -screen_file which is the only non-supported argument in SIPp < 3.5.0. Also, the description was included in the README.

goodboy commented 6 years ago

@y-luis awesome nice work :+1: Mind rebasing this onto master now that #41 is in - get a clean run and this is likely good to go!

y-luis-rojo commented 6 years ago

Thanks, @tgoodlet. Merged #41.

goodboy commented 6 years ago

@y-luis yeah see my similar quiff in #38 but you should rebase out the history from the CI fix branch so they don't show up here.

Thanks!

y-luis-rojo commented 6 years ago

git rebase done.