-
Notifications
You must be signed in to change notification settings - Fork 25.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove the need for _UNRELEASED suffix in versions #24798
Changes from 5 commits
c3a2dd7
e9c0251
cfa8154
3b9d569
405deef
fa1a4e0
a2376ed
43a11b3
1b4223b
ab38eb5
8db2ced
581abea
351a970
a9a9d03
9989cd3
089d2b2
bb62229
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -290,7 +290,7 @@ task('verifyVersions') { | |
} | ||
Set<String> knownVersions = new TreeSet<>(xml.versioning.versions.version.collect { it.text() }.findAll { it ==~ /\d\.\d\.\d/ }) | ||
|
||
// Limit the known versions to those that should be wire compatible | ||
// Limit the known versions to those that should be index compatible | ||
String currentVersion = versions.elasticsearch.minus('-SNAPSHOT') | ||
int prevMajor = Integer.parseInt(currentVersion.split('\\.')[0]) - 1 | ||
if (prevMajor == 4) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This logic only needs to exist on the 5.x branches. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. I figure it helps to keep them in sync so backporting is easier. |
||
|
@@ -305,9 +305,11 @@ task('verifyVersions') { | |
* as possible after release. */ | ||
Set<String> actualVersions = new TreeSet<>( | ||
indexCompatVersions | ||
.findAll { false == it.unreleased } | ||
.findAll { false == it.snapshot } | ||
.collect { it.toString() }) | ||
|
||
// TODO this is almost certainly going to fail on 5.4 when we release 5.5.0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I want to deal with this in a followup. This change is big enough as is. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you mean by "fail on 5.4"? The 5.4 branch? I think we need to filter the actualVersions for things less than the current version? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe so. I added this here because I noticed it here. I think it'd be nice to fix it in a separate PR though because it isn't really related to this. |
||
|
||
// Finally, compare! | ||
if (!knownVersions.equals(actualVersions)) { | ||
throw new GradleException("out-of-date versions\nActual :" + | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -74,15 +74,17 @@ public class Version implements Comparable<Version> { | |
public static final Version V_5_3_2 = new Version(V_5_3_2_ID, org.apache.lucene.util.Version.LUCENE_6_4_2); | ||
public static final int V_5_4_0_ID = 5040099; | ||
public static final Version V_5_4_0 = new Version(V_5_4_0_ID, org.apache.lucene.util.Version.LUCENE_6_5_0); | ||
public static final int V_5_5_0_ID_UNRELEASED = 5050099; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I almost certainly should have waited to make this change as a purely mechanical followup. It nasty to backport. But I already bit this pill so I feel like I have to swallow it now.... |
||
public static final Version V_5_5_0_UNRELEASED = new Version(V_5_5_0_ID_UNRELEASED, org.apache.lucene.util.Version.LUCENE_6_5_0); | ||
public static final int V_6_0_0_alpha1_ID_UNRELEASED = 6000001; | ||
public static final Version V_6_0_0_alpha1_UNRELEASED = | ||
new Version(V_6_0_0_alpha1_ID_UNRELEASED, org.apache.lucene.util.Version.LUCENE_7_0_0); | ||
public static final int V_6_0_0_alpha2_ID_UNRELEASED = 6000002; | ||
public static final Version V_6_0_0_alpha2_UNRELEASED = | ||
new Version(V_6_0_0_alpha2_ID_UNRELEASED, org.apache.lucene.util.Version.LUCENE_7_0_0); | ||
public static final Version CURRENT = V_6_0_0_alpha2_UNRELEASED; | ||
public static final int V_5_4_1_ID = 5040199; | ||
public static final Version V_5_4_1 = new Version(V_5_4_1_ID, org.apache.lucene.util.Version.LUCENE_6_5_0); | ||
public static final int V_5_5_0_ID = 5050099; | ||
public static final Version V_5_5_0 = new Version(V_5_5_0_ID, org.apache.lucene.util.Version.LUCENE_6_5_0); | ||
public static final int V_6_0_0_alpha1_ID = 6000001; | ||
public static final Version V_6_0_0_alpha1 = | ||
new Version(V_6_0_0_alpha1_ID, org.apache.lucene.util.Version.LUCENE_7_0_0); | ||
public static final int V_6_0_0_alpha2_ID = 6000002; | ||
public static final Version V_6_0_0_alpha2 = | ||
new Version(V_6_0_0_alpha2_ID, org.apache.lucene.util.Version.LUCENE_7_0_0); | ||
public static final Version CURRENT = V_6_0_0_alpha2; | ||
|
||
// unreleased versions must be added to the above list with the suffix _UNRELEASED (with the exception of CURRENT) | ||
|
||
|
@@ -97,12 +99,14 @@ public static Version readVersion(StreamInput in) throws IOException { | |
|
||
public static Version fromId(int id) { | ||
switch (id) { | ||
case V_6_0_0_alpha2_ID_UNRELEASED: | ||
return V_6_0_0_alpha2_UNRELEASED; | ||
case V_6_0_0_alpha1_ID_UNRELEASED: | ||
return V_6_0_0_alpha1_UNRELEASED; | ||
case V_5_5_0_ID_UNRELEASED: | ||
return V_5_5_0_UNRELEASED; | ||
case V_6_0_0_alpha2_ID: | ||
return V_6_0_0_alpha2; | ||
case V_6_0_0_alpha1_ID: | ||
return V_6_0_0_alpha1; | ||
case V_5_5_0_ID: | ||
return V_5_5_0; | ||
case V_5_4_1_ID: | ||
return V_5_4_1; | ||
case V_5_4_0_ID: | ||
return V_5_4_0; | ||
case V_5_3_2_ID: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -308,7 +308,7 @@ public void readFrom(StreamInput in) throws IOException { | |
indicesOptions = IndicesOptions.readIndicesOptions(in); | ||
type = in.readOptionalString(); | ||
source = in.readString(); | ||
if (in.getVersion().before(Version.V_6_0_0_alpha1_UNRELEASED)) { // TODO change to V_5_3 once backported | ||
if (in.getVersion().before(Version.V_6_0_0_alpha1)) { // TODO change to V_5_3 once backported | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This TODO worries me some.... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jaymode, should this change? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes it should change. I'll open a PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Opened #24835 for both |
||
// we do not know the format from earlier versions so convert if necessary | ||
source = XContentHelper.convertToJson(new BytesArray(source), false, false, XContentFactory.xContentType(source)); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -475,7 +475,7 @@ public void readFrom(StreamInput in) throws IOException { | |
cause = in.readString(); | ||
name = in.readString(); | ||
|
||
if (in.getVersion().onOrAfter(Version.V_6_0_0_alpha1_UNRELEASED)) { | ||
if (in.getVersion().onOrAfter(Version.V_6_0_0_alpha1)) { | ||
indexPatterns = in.readList(StreamInput::readString); | ||
} else { | ||
indexPatterns = Collections.singletonList(in.readString()); | ||
|
@@ -487,7 +487,7 @@ public void readFrom(StreamInput in) throws IOException { | |
for (int i = 0; i < size; i++) { | ||
final String type = in.readString(); | ||
String mappingSource = in.readString(); | ||
if (in.getVersion().before(Version.V_6_0_0_alpha1_UNRELEASED)) { // TODO change to V_5_3_0 once backported | ||
if (in.getVersion().before(Version.V_6_0_0_alpha1)) { // TODO change to V_5_3_0 once backported | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jaymode here too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And maybe we're missing something in a mixed cluster test? Maybe we just aren't running it yet..... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should also change. We do not catch this in our BWC/mixed cluster tests since these checks just guard the need to do content type auto-detection. In versions > 5.3.0 we always send JSON so we do not need to autodetect, but this check being wrong kept the auto detection in place when it wasn't necessary. |
||
// we do not know the incoming type so convert it if needed | ||
mappingSource = | ||
XContentHelper.convertToJson(new BytesArray(mappingSource), false, false, XContentFactory.xContentType(mappingSource)); | ||
|
@@ -512,7 +512,7 @@ public void writeTo(StreamOutput out) throws IOException { | |
super.writeTo(out); | ||
out.writeString(cause); | ||
out.writeString(name); | ||
if (out.getVersion().onOrAfter(Version.V_6_0_0_alpha1_UNRELEASED)) { | ||
if (out.getVersion().onOrAfter(Version.V_6_0_0_alpha1)) { | ||
out.writeStringList(indexPatterns); | ||
} else { | ||
out.writeString(indexPatterns.size() > 0 ? indexPatterns.get(0) : ""); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is weird to me this task is on core. Could it just be in the root project? And create a precommit task there which depends on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'd be easy enough to just move the whole thing into the root project. Is that what you'd prefer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rereading, yes, that looks like what you'd prefer. I'll do that now.