Skip to content

Move latest major version parsing into versions#2131

Closed
Copilot wants to merge 3 commits into
masterfrom
copilot/fix-code-review-suggestion
Closed

Move latest major version parsing into versions#2131
Copilot wants to merge 3 commits into
masterfrom
copilot/fix-code-review-suggestion

Conversation

Copilot AI commented May 29, 2026

Copy link
Copy Markdown
Contributor

This addresses the review feedback to centralize distribution major-version parsing in esrally.utils.versions rather than keeping ad hoc parsing in the provisioner. The change also adds focused coverage for the extracted helper.

  • Extract version parsing helper

    • Added latest_supported_major(version) to esrally.utils.versions.
    • Preserves the special-case handling for serverless.
    • Uses Version.from_string(...).major for versioned distributions.
  • Align provisioner with shared logic

    • Keeps ElasticsearchInstaller on the shared versions API instead of local parsing logic.
    • Reduces duplication in the provisioning path.
  • Add focused coverage

    • Added unit coverage for:
      • serverless
      • standard semantic versions
      • snapshot versions
def latest_supported_major(version):
    if version.version() == "serverless":
        return "serverless"
    return Version.from_string(version.version()).major

Copilot AI changed the title [WIP] Fix code according to review comment Move latest major version parsing into versions May 29, 2026
Copilot AI requested a review from gareth-ellis May 29, 2026 06:52
@gareth-ellis

Copy link
Copy Markdown
Member

@copilot which comment triggered this PR to be raised? I cant see a connection

Copilot AI commented May 29, 2026

Copy link
Copy Markdown
Contributor Author

@copilot which comment triggered this PR to be raised? I cant see a connection

I couldn’t find a specific visible review comment on this PR that directly triggered it. This follow-up was intended to address earlier feedback about centralizing the major-version parsing in esrally.utils.versions rather than keeping it in the provisioner, but that link isn’t clear from the PR thread.

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