feat(perf): Added new CLI flags for iterations + updated docs #258
Conversation
Co-authored-by: Prithvi Shahi <shahi.prithvi@gmail.com>
There was a problem hiding this comment.
I don't think this pull request was ready for merge. This is partially on me, due to my delayed review.
I don't think the --iterations flag is intuitive. See comment below. Also it is likely going away again with #261, as upload and download bandwidth tests will likely only run as a single iteration.
| @@ -26,13 +33,28 @@ Benchmark results can be visualized with https://observablehq.com/@libp2p-worksp | |||
| 5. `CLIENT_IP=$(terraform output -raw client_ip)` | |||
| 6. `SERVER_IP=$(terraform output -raw server_ip)` | |||
|
|
|||
| ### Build and run implementations | |||
| **Notes** | |||
| - You may need to reset the infrastructure if you encounter any errors, you can do that by running `terraform destroy` and then `terraform apply`. | |||
There was a problem hiding this comment.
I don't think this is a good general hint. In the most cases terraform should be able to reconciliate any changes without a previous terraform destroy. On a high level, I am hesitant to give general terraform advice here. I think that is much better placed in the official terraform documentation.
| - Logging MUST go to `stderr`. | ||
| - Measurement output is printed to **stdout** as JSON in the form of: |
There was a problem hiding this comment.
Nit pick. Either use monospace or bold.
| │ with module.short_lived_server[0].aws_instance.perf, | ||
| │ on ../../modules/short_lived/main.tf line 15, in resource "aws_instance" "perf": | ||
| │ 15: resource "aws_instance" "perf" { | ||
| ``` |
There was a problem hiding this comment.
Let's add some context here along the lines of the below.
This implies that you haven't deployed the long-living infrastructure (e.g. launch template) on your AWS account. To do so along with each short-lived deployment, you can set the
long_lived_enabled...
|
How about replacing the |
While integrating the js impl of perf I encountered some obstacles in running this infrastructure as well as adding and testing the perf implementation. These are some improvements I thought would help new users of perf
Related #222