search

markormesher / iperf-prometheus-collector

GNU Affero General Public License v3.0
3 stars 1 forks source link

Options and protocol env #27

Open gberche-orange opened 3 days ago

gberche-orange commented 3 days ago

Add support for specifying options and protocol (udp/tcp) as new options Also adds github workflow to publish latest commit in main branch as latest

fix https://github.com/markormesher/iperf-prometheus-collector/issues/24 fix https://github.com/markormesher/iperf-prometheus-collector/issues/26

For udp protocol, only the received metrics are emitted (as sent metrics are missing from json report), and an additional lost_packet metrics is emitted.

markormesher commented 3 days ago

Hey @gberche-orange, thanks for opening this PR and the handful of issues over the last week. I'll take proper look through them this week, just wanted to let you know I'm not ignoring you and I will respond!

gberche-orange commented 3 days ago

Great, thanks a lot @markormesher !

markormesher commented 3 days ago

I'll review this properly after I deal with the upgrades I mentioned in #24, but a couple of initial high level comments: this project already uses CircleCI for releases so there's no need to add GitHub Actions, and the error handling for the protocol argument should happen before trying to run a test (and should look like the other error handling rather than throwing a bare exception).

gberche-orange commented 2 days ago

this project already uses CircleCI for releases so there's no need to add GitHub Actions

After a second throught, it might still be convenient to accept a github workflow in the repo even if github workflows are not enabled: it might be convenient for contributors to easily test their changes in their fork and generate an OCI image without going through the circle ci onboarding.

markormesher commented 2 days ago

It could be, but it creates two sources of truth for how the project is built and published. CircleCI onboarding is trivial, as is building the images locally while you're developing, so I prefer keeping the project simpler for now.

gberche-orange commented 3 hours ago

CircleCI onboarding is trivial, as is building the images locally while you're developing, so I prefer keeping the project simpler for now.

I'm trying to see if the project circle ci project is public for users/contributors to review build logs.

I'm currently unable to discover the circle ci organization url or build jobs uris.

Circle ci seems to support public ci since recently according to https://circleci.com/changelog/projects-with-public-github-app-triggers-are-now-recognized-as-open-source/

Some more details about circleci onboarding:

I tried to login to circleci to see if there are publicly available builds for this repo, but I'm hitting circleci restrictions that it needs access to private repos that my company policy forbids

See more in https://discuss.circleci.com/t/circleci-now-has-scope-to-see-my-private-repositories-after-simply-logging-in-via-github-oauth/43549

I've worked around it with email login instead.

ps: not an urgent/important matter though

markormesher commented 3 hours ago

Can you see the details on a link like https://app.circleci.com/jobs/github/markormesher/iperf-prometheus-collector/325? In Incognito mode I can still see the builds and the logs. You can get those links from the status reports on commits or PRs.

gberche-orange commented 3 hours ago

Sorry, my bad, I have overlooked the checks in the PR when I checked image expanding it, I can properly access the build, but logs seems missing image image