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

add possibility to specify server-id #21

Merged
merged 4 commits into from
Apr 9, 2020

Conversation

Hades32
Copy link
Contributor

@Hades32 Hades32 commented Apr 5, 2020

To be have less variance in the measurements I'd like to always use the same server and not rely on the auto-detection which sometimes selects pretty bad servers.

Not much python experience here, so hope the PR is ok :)

@Hades32
Copy link
Contributor Author

Hades32 commented Apr 5, 2020

BTW, I opened this PR originally, because the official speedtest CLI is able to measure >900MBit/s for my line. But this version here only get about 280 MBit/s . Both running on my Pine64 (arm64) board. Maybe you have an idea, why...

@jeanralphaviles
Copy link
Owner

BTW, I opened this PR originally, because the official speedtest CLI is able to measure >900MBit/s for my line. But this version here only get about 280 MBit/s . Both running on my Pine64 (arm64) board. Maybe you have an idea, why...

Nice internet :). I haven't seen any differences in speed reported by speedtest-cli and prometheus_speedtest; at least at the 85 MiB I'm getting. We're just a thin wrapper around the tool.

It could be the client options we're setting. This server flag should help, thank you.

@Hades32
Copy link
Contributor Author

Hades32 commented Apr 7, 2020

Ah I probably should have been more specific. When I was referring to "the official speedtest CLI" I did not mean the python speedtest-cli project, but the CLI provided by speedtest.net which is a binary (maybe C?). But yes, probably then it would be a problem in your underlying library and not in your code. Probably should open an issue there.

Copy link
Owner

@jeanralphaviles jeanralphaviles left a comment

Choose a reason for hiding this comment

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

Just these couple None vs [] nits then LGTM.

prometheus_speedtest/prometheus_speedtest.py Outdated Show resolved Hide resolved
prometheus_speedtest/prometheus_speedtest.py Outdated Show resolved Hide resolved
@Hades32
Copy link
Contributor Author

Hades32 commented Apr 8, 2020

Thanks for your feedback. Incorporated all your requested changes.

BTW, I reported the wrong measurements downstream, but was not very helpful: https://github.com/sivel/speedtest-cli/issues/716#issuecomment-610788176

@jeanralphaviles jeanralphaviles merged commit 3ae293b into jeanralphaviles:master Apr 9, 2020
@jeanralphaviles
Copy link
Owner

Thank you!

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.

None yet

2 participants