Skip to content

Removes random parameter#140266

Merged
joshua-adams-1 merged 10 commits intomainfrom
random-compatible-version-random-param
Jan 19, 2026
Merged

Removes random parameter#140266
joshua-adams-1 merged 10 commits intomainfrom
random-compatible-version-random-param

Conversation

@joshua-adams-1
Copy link
Contributor

A number of utils methods in TransportVersionUtils, MlConfigVersionUtils, VersionUtils, IndexVersionUtils and TransformConfigVersionUtils had a random parameter that was only ever passed in as Lucene.test.random(). This removes the unnecessary parameter.

A number of utils methods in TransportVersionUtils,
MlConfigVersionUtils, VersionUtils, IndexVersionUtils
and TransformConfigVersionUtils had a random parameter
that was only ever passed in as Lucene.test.random().
This removes the unnecessary parameter.
@joshua-adams-1 joshua-adams-1 marked this pull request as ready for review January 7, 2026 16:12
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination)

@elasticsearchmachine elasticsearchmachine added the Team:Distributed Coordination (obsolete) Meta label for Distributed Coordination team. Obsolete. Please do not use. label Jan 7, 2026
Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

Ok with the ones that remove the random() argument from method calls, just a little supicious about the ones that remove actual methods. Let's see what core/infra say about #140222 - maybe there's some good reason why we were passing a Random around everywhere here?

@joshua-adams-1
Copy link
Contributor Author

I was only removing the method because it was overloading a parameterless one, and so me removing the parameter made a duplicate method. E.g:

/** Returns a random {@link TransportVersion} from all available versions. */
    public static TransportVersion randomVersion() {
        return RandomPicks.randomFrom(random(), allReleasedVersions());
    }
    /** Returns a random {@link TransportVersion} from all available versions without the ignore set */
    public static TransportVersion randomVersion(Set<TransportVersion> ignore) {
        return ESTestCase.randomFrom(allReleasedVersions().stream().filter(v -> ignore.contains(v) == false).collect(Collectors.toList()));
    }

    /** Returns a random {@link TransportVersion} from all available versions. */
    public static TransportVersion randomVersion(Random random) {
        return RandomPicks.randomFrom(random, allReleasedVersions());
    }

by removing Random random as a parameter becomes:

/** Returns a random {@link TransportVersion} from all available versions. */
    public static TransportVersion randomVersion() {
        return RandomPicks.randomFrom(random(), allReleasedVersions());
    }
    /** Returns a random {@link TransportVersion} from all available versions without the ignore set */
    public static TransportVersion randomVersion(Set<TransportVersion> ignore) {
        return ESTestCase.randomFrom(allReleasedVersions().stream().filter(v -> ignore.contains(v) == false).collect(Collectors.toList()));
    }

    /** Returns a random {@link TransportVersion} from all available versions. */
    public static TransportVersion randomVersion() {
        return RandomPicks.randomFrom(random(), allReleasedVersions());
    }

which has a duplicate of the existing, hence the whole method got removed

@joshua-adams-1
Copy link
Contributor Author

To clarify, I did not remove any method that according to IntelliJ "wasn't used anywhere". That was reserved for #140222 so it could be a separate discussion

@joshua-adams-1
Copy link
Contributor Author

As a note, #140222 has been merged

Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

Ok LGTM

@joshua-adams-1 joshua-adams-1 added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Jan 12, 2026
@joshua-adams-1 joshua-adams-1 removed the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Jan 16, 2026
@joshua-adams-1 joshua-adams-1 merged commit 3775bfa into main Jan 19, 2026
37 checks passed
@joshua-adams-1 joshua-adams-1 deleted the random-compatible-version-random-param branch January 19, 2026 13:01
spinscale pushed a commit to spinscale/elasticsearch that referenced this pull request Jan 21, 2026
A number of utils methods in TransportVersionUtils,
MlConfigVersionUtils, VersionUtils, IndexVersionUtils
and TransformConfigVersionUtils had a random parameter
that was only ever passed in as Lucene.test.random().
This removes the unnecessary parameter.
@repantis repantis added the :Distributed/Distributed A catch all label for anything in the Distributed Area. Please avoid if you can. label Jan 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed/Distributed A catch all label for anything in the Distributed Area. Please avoid if you can. >non-issue serverless-linked Added by automation, don't add manually Team:Distributed Coordination (obsolete) Meta label for Distributed Coordination team. Obsolete. Please do not use. v9.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants