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

added flashlight-tester, to be used by pinger #1464

Merged
merged 4 commits into from
Dec 16, 2024
Merged

added flashlight-tester, to be used by pinger #1464

merged 4 commits into from
Dec 16, 2024

Conversation

reflog
Copy link
Contributor

@reflog reflog commented Dec 11, 2024

please see tester/README.md for details

@reflog reflog force-pushed the reflog/tester branch 2 times, most recently from 099d385 to e2b4cc1 Compare December 11, 2024 15:54
Copy link
Contributor

@hwh33 hwh33 left a comment

Choose a reason for hiding this comment

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

A minor point of confusion over naming (feel free to ignore), but this looks good.

tester/README.md Outdated
- `TOKEN`: The token to use.
- `RUN_ID`: The run id to use. It will be added to honeycomb traces as value of attribute `pinger-id`. (you can use it for looking up traces for the specific run)
- `TARGET_URL`: The target url to use. This is the url that will be pinged through flashlight.
- `OUTPUT`: The output path to use. This is the path where the output will be written to (proxies.yaml, global.yaml, etc). You can place proxies.yaml there to use it instead of fetching.
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a lot of language in here about the "output directory" which in reality appears to be the config directory. Why not just call it the config directory? That's what the team is used to calling that folder. Given that it holds config, it's also more of an input directory which adds some confusion to the new lingo IMO.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see that you are writing output files to the config directory. You could either (a) define a new directory for that, accepting both an OUTPUT_DIR and CONFIG_DIR (which could be the same) or (b) just state that the program will write output files to the config directory.

@reflog reflog merged commit ff855c4 into main Dec 16, 2024
1 check passed
@reflog reflog deleted the reflog/tester branch December 16, 2024 16:41
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