Skip to content

infer Vitess version for admin executions#673

Open
mhamza15 wants to merge 2 commits intomainfrom
admin-version
Open

infer Vitess version for admin executions#673
mhamza15 wants to merge 2 commits intomainfrom
admin-version

Conversation

@mhamza15
Copy link
Contributor

@mhamza15 mhamza15 commented Mar 2, 2026

What kind of change does this PR introduce?

Admin-triggered /api/executions/add requests were enqueueing jobs with an empty Vitess version while benchmark configuration is version-gated. For OLAP workloads, exec-vitess-config entries such as mysql_default_workload=OLAP under version 14+ could be skipped, so admin runs took OLTP execution paths instead of OLAP streaming behavior. The handler also had two missing early returns in error branches, so execution could continue after invalid PR metadata or missing SHA/PR input.

This change infers Vitess version before queueing by resolving a reference SHA (PR base for PR requests, requested SHA otherwise), refreshing the local Vitess clone, calling git.GetVersionForCommitSHA, and passing the resolved version into createSimpleExecutionQueueElement.

Issue Number:

  • Closes #___
  • Related to #___
  • Others?

Screenshots/videos:

Before After

If relevant, did you update the documentation?

Summary

Does this PR introduce a breaking change?

Admin-triggered `/api/executions/add` requests were enqueueing jobs with an empty Vitess version while benchmark configuration is version-gated. For OLAP workloads, `exec-vitess-config` entries such as `mysql_default_workload=OLAP` under version 14+ could be skipped, so admin runs took OLTP execution paths instead of OLAP streaming behavior. The handler also had two missing early returns in error branches, so execution could continue after invalid PR metadata or missing SHA/PR input.

This change infers Vitess version before queueing by resolving a reference SHA (PR base for PR requests, requested SHA otherwise), refreshing the local Vitess clone, calling `git.GetVersionForCommitSHA`, and passing the resolved version into `createSimpleExecutionQueueElement`. It also validates unknown workloads up front and restores explicit early returns, so admin and cron paths now apply version-gated flags consistently and follow clearer control flow.
@mhamza15 mhamza15 self-assigned this Mar 2, 2026
@mhamza15 mhamza15 requested a review from frouioui as a code owner March 2, 2026 21:33
Signed-off-by: Mohamed Hamza <mhamza@fastmail.com>
Copy link

@timvaillancourt timvaillancourt left a comment

Choose a reason for hiding this comment

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

@mhamza15 some CI is failing, but LGTM overall

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.

2 participants