Skip to content
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

Update throttlenetwork.md #1575

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

gopikrishnan-p
Copy link

updated networkProfile parameters to reflect the correct values. They were missing Throughput

What?

updated networkProfile parameters to reflect the correct values. They were missing Throughput keyword.

Checklist

  • I have used a meaningful title for the PR.
  • I have described the changes I've made in the "What?" section above.
  • I have performed a self-review of my changes.
  • I have run the npm start command locally and verified that the changes look good.
  • I have made my changes in the docs/sources/next folder of the documentation.
  • I have reflected my changes in the docs/sources/v0.50.x folder of the documentation.
  • I have reflected my changes in the relevant folders of the two previous k6 versions of the documentation (if still applicable to previous versions).

Related PR(s)/Issue(s)

updated networkProfile parameters to reflect the correct values. They were missing Throughput
@CLAassistant
Copy link

CLAassistant commented Apr 30, 2024

CLA assistant check
All committers have signed the CLA.

@gopikrishnan-p

This comment was marked as resolved.

Comment on lines +14 to +15
| networkProfile.downloadThroughput | number | `-1` | Maximal aggregated download throughput (bytes/sec). `-1` disables download throttling. |
| networkProfile.uploadThroughput | number | `-1` | Maximal aggregated upload throughput (bytes/sec). `-1` disables upload throttling. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the change. The correct values should be download and upload. What's the reason for requesting the documentation change?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ankur22,
I was able to use both download and downloadThroughput values. Some of my older scripts which uses 'download' were failing for some reason. But now I see both are working while I did the testing. So if there are notes saying 'downloadThroughput' is deprecated or shouldn't be used it would be great!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was a brief moment where downloadThroughput and uploadThroughput were part of a PR when the API was being built, but they were never released as supported fields for this API. I would recommend changing your scripts to work with download and upload.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants