Midsummer Milestone #3
Conversation
…and a flask file name
…ator instead of httpclient
twood02
left a comment
twood02
left a comment
There was a problem hiding this comment.
sorry for the delay reviewing this! You have lots of good code here! My comments are mainly small cleanup fixes to make the code more readable and consistent.
The main things to change/investigate are:
- Add a README.md file with basic documentation for how to use your tool (including any library dependencies that need to be installed, how to set workload parameters, etc)
- be sure you know the implications of using setTimeout instead of HighResolution timer
- adjust python scripts to take file names as arguments
| timeStamps[num_rquests] = cumulative_time; | ||
| num_rquests++; | ||
| setTimeout(() => this.makeRequest(), cumulative_time); // sets a timer for all the requests | ||
| } |
There was a problem hiding this comment.
ok now I understand what you meant in the meeting when you said you setup all the timeouts at the start. Your new code uses the setTimeout function to setup the timers while the old code used the HighResolutionTimer class. Is there a reason for this? Based on the name, it sounds like "HighResolutionTimer" might be more accurate. I don't know how precise the normal setTimeout is.
There was a problem hiding this comment.
The HighResolutionTimer is a class created by the loadtest developers. I looked into the code and it seems to use delays along with setTimeout. The constructor takes in two arguments a callback function and a delay period and then calls the callback function once every time a delay period passes, which seems like it is a way to send the requess at a fixed rate. The code for the class is found in the lib/hrtimer.js.
|
@twood02 Hi professor, These new commits now include the code for the client policies, the readme, and the fixes based on the previous feedback. |
3 participants
@twood02 Hi professor, I cleaned up the scripts a little and added some comments, Amanda and Sarah have sent the graphing scripts, which are also included in this pull request.