Re-enables auto benchmarking / acceptance testing #78
Conversation
…able-auto-benchmarks
.github/workflows/auto-benchmark.yml
Outdated
| - name: run acceptance test | ||
| run: | | ||
| export BRANCH_NAME=$(echo $GITHUB_REF | awk -F'/' '{print $3}') | ||
| export BENCHMARK_API_KEY_PROD=${{ secrets.BENCHMARK_API_KEY_PROD }} |
There was a problem hiding this comment.
Can you use the env feature of the GH workflow for BENCHMARK_API_KEY_PROD and SLACK_URL_DEV_SCIENCE?
There was a problem hiding this comment.
What do you mean? 🤔
This is necessary to actually surface the secret as an environment variable in the bash sense. env of Github is used to pass environment variables between steps and/or jobs.
I don't think you need to worry about the secrets leaking this way. As far as I can tell this is the intended way of things to work. Github removes the secrets from the log output (see image).
I can rewrite the script to accept these as arguments via argparse (bypassing environment variables), but the call to it would still be logged like this, I think:
python .nextmv/benchmark.py --api-key "***" --slack-url "***"There was a problem hiding this comment.
No my comment just referred to using the yaml env key within a step instead of export. I thought that would be the same. But no strong opinion, export obviously also works
e.g.
env:
BENCHMARK_ACCOUNT_ID: ${{ vars.BENCHMARK_ACCOUNT_ID }}
BENCHMARK_API_KEY_PROD: ${{ secrets.BENCHMARK_API_KEY_PROD }}There was a problem hiding this comment.
Sure, works for me. Does indeed look a little nicer. 😊
| APP_ID = "nextroute-bench" | ||
| API_KEY = os.environ["BENCHMARK_API_KEY_PROD"] | ||
| SLACK_WEBHOOK = os.getenv("SLACK_URL_DEV_SCIENCE", None) | ||
| ACCOUNT_ID = os.getenv("BENCHMARK_ACCOUNT_ID", None) | ||
| BRANCH_NAME = os.getenv("BRANCH_NAME", None) |
There was a problem hiding this comment.
Could have used nextmv.Options for this 👻
There was a problem hiding this comment.
Haha, fair. I suppose I should get used to it more. 😊
Description
Important
The selected acceptance test criteria were just for testing and need to be revised.
Re-enables auto-benchmarking. This will automatically run a short acceptance test pre-merge to fend off major performance regressions.
The acceptance test currently runs on every push and will be used to block merges if it fails. Furthermore, the test runs again post-merge to ping Slack with the result and update the baseline instance for future runs.
Changes
.nextmv/benchmark.py.auto-benchmark.yml.cmd/main.gotest entrypoint to work with benchmark inputs (they declare an override max duration).Resolves ENG-5633