feat: experiment run does what the run-experiment action does - #80
Merged
Merged
Conversation
The steadybit/run-experiment GitHub Action has its own API client; to run it on the CLI instead, `experiment run` needs what it offers: - --expect-state passes once the run reaches a state, which need not be its end (RUNNING), and fails when it ends in another; --expect-reason also requires the reason. Without them a run has to complete, as before. - --expectation-retries runs the experiment again when a run did not end as expected, --expectation-retry-interval apart. - --busy-retries waits and tries again while another experiment runs, both when the platform refuses the run and when it cancels it right after accepting it, and never starts it in parallel instead, which --yes would do. The platform's rule spans teams, so this matters in a shared tenant. - --external-id without --template runs the experiment with that external id. - The JSON report gives each run's apiLocation, the action's executionUrl. - With --retries, the last attempt is kept on the platform, as the action does, so a run shows what was wrong. "Another experiment running" is now told apart before validation errors: the platform answers both with 422, so with --retries the former used to be retried as a validation error. Both run paths share one function that starts, waits and retries. The weekly platform test covers the new flags.
A change to the platform test can use flags the latest release does not have yet, so on a pull request the CLI is built from the branch.
The expected-state check threw the CLI's output away, so a failure in CI said nothing about why. It now runs through exits_with, which prints it, and the check on a run ending otherwise asserts the message, not only the exit code.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Step 1 of moving
steadybit/run-experimentonto the CLI:experiment rungains what the action offers. Step 2 turns the action into a thin wrapper that installs the CLI and maps its inputs and outputs, with the same inputs and outputs, so customers change nothing.Parity, input by input
experimentKey-kexternalId--external-idwithout--template. It fails if no experiment, or more than one, has that external id.parallel--allowParallelmaxRetries(another running, 30 s apart)--busy-retries/--busy-retry-interval(default 30 s). They take priority over--yes, so the run is never started in parallel instead. They also cover the run the platform accepts and then cancels "because another experiment was running".expectedState/expectedReason--expect-state/--expect-reason. They pass once the run reaches the state, as the action does (RUNNINGpasses before the end), and fail when the run ends in another state. The reason must match exactly.maxRetriesOnExpectationFailure/delayBetweenRetriesOnExpectationFailure--expectation-retries/--expectation-retry-interval. They run the experiment again.maxRetriesOnValidationFailure/delayBetweenRetriesOnValidationFailure--retries/--retryInterval. Now the last attempt is kept on the platform, as the action does. Before, no attempt was kept.executionId,executionState,executionReason,executionUrl--report run.json:id,state,reason, and newapiLocation, the run'sLocation, which is what the action outputs asexecutionUrl. The reason falls back to the legacyfailureReason, as the action reads it.Without the new flags, nothing changes: a run still has to complete, with the same messages.
Also fixed: the platform answers both "another experiment running" and validation errors with 422. With
--retries, the first used to be retried as a validation error. The problem type is now checked first.Found on dev: the platform's "another experiment running" rule spans teams. A run of team
CLIwas cancelled because anADMrun was going. So--busy-retriesmatters in a shared tenant. It's also why the weekly platform test only checks that a retry happens, not that the platform eventually becomes free.Testing
Unit tests:
allowParallel=true;apiLocation;-k;--no-wait;go test -race ./...andgo vet(alsoGOOS=windows) pass.Live on dev, team
CLI, wait-only experiments, all deleted:--external-id … --expect-state RUNNINGpassed after 6 s, and the report hadRUNNINGandapiLocation;--expect-state FAILEDon a completing run failed with "completed, but failed was expected";--busy-retries 4waited through two refusals while another run went, then completed, with--yes.e2e/platform.shnow covers the external id, the expected state and busy retries. All 14 checks pass locally against dev.