Skip to content

Commit 9e08be5

Browse files
jordan-powersjordan-powers
committed
Merge remote-tracking branch 'upstream/main' into logs-message-default-sort
2 parents 33e46ab + 0050679 commit 9e08be5

File tree

9 files changed

+476
-82
lines changed

9 files changed

+476
-82
lines changed

‎build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionValidationFuncTest.groovy‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -312,8 +312,7 @@ class TransportVersionValidationFuncTest extends AbstractTransportVersionFuncTes
312312
result.task(":myserver:validateTransportVersionResources").outcome == TaskOutcome.SUCCESS
313313
}
314314

315-
def "modification checks use PR base branch in CI"() {
316-
given:
315+
void baseBranchSetup() {
317316
// create 9.1 branch
318317
execute("git checkout -b 9.1")
319318
// setup 9.1 upper bound as if 9.2 was just branched, so no patches yet
@@ -342,10 +341,27 @@ class TransportVersionValidationFuncTest extends AbstractTransportVersionFuncTes
342341
currentUpperBoundName = '9.1'
343342
}
344343
"""
344+
}
345+
346+
def "modification checks use PR base branch in CI"() {
347+
given:
348+
baseBranchSetup()
349+
350+
when:
351+
def result = gradleRunner("validateTransportVersionResources")
352+
.withEnvironment(Map.of("BUILDKITE_PULL_REQUEST_BASE_BRANCH", "9.1", "BUILDKITE_BRANCH", "foobar")).build()
353+
354+
then:
355+
result.task(":myserver:validateTransportVersionResources").outcome == TaskOutcome.SUCCESS
356+
}
357+
358+
def "modification checks use buildkite base branch in CI if not in PR"() {
359+
given:
360+
baseBranchSetup()
345361

346362
when:
347363
def result = gradleRunner("validateTransportVersionResources")
348-
.withEnvironment(Map.of("BUILDKITE_PULL_REQUEST_BASE_BRANCH", "9.1")).build()
364+
.withEnvironment(Map.of("BUILDKITE_BRANCH", "9.1")).build()
349365

350366
then:
351367
result.task(":myserver:validateTransportVersionResources").outcome == TaskOutcome.SUCCESS

‎build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionResourcesService.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,12 @@ private String findUpstreamRef() {
287287
// default the branch name to look at to that which a PR in CI is targeting
288288
String branchName = System.getenv("BUILDKITE_PULL_REQUEST_BASE_BRANCH");
289289
if (branchName == null || branchName.strip().isEmpty()) {
290-
branchName = "main";
290+
// fallback to the local branch being tested in CI
291+
branchName = System.getenv("BUILDKITE_BRANCH");
292+
if (branchName == null || branchName.strip().isEmpty()) {
293+
// fallback to main if we aren't in CI
294+
branchName = "main";
295+
}
291296
}
292297
List<String> remoteNames = List.of(remotesOutput.split("\n"));
293298
if (remoteNames.contains(UPSTREAM_REMOTE_NAME) == false) {

‎docs/changelog/136577.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 136577
2+
summary: Clean up inference indices on failed endpoint creation
3+
area: Machine Learning
4+
type: bug
5+
issues:
6+
- 123726

‎docs/changelog/136649.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 136649
2+
summary: Fix append processor `ignore_empty_values` edge case
3+
area: Ingest Node
4+
type: bug
5+
issues: []

‎modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/380_append_edge_cases.yml‎

Lines changed: 236 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,236 @@
1+
---
2+
setup:
3+
- requires:
4+
cluster_features: [ "ingest.append.ignore_empty_values_fix" ]
5+
reason: "These tests all verify that a bug in ignore_empty_values has been fixed"
6+
- do:
7+
ingest.put_pipeline:
8+
id: "test-pipeline-1"
9+
body: >
10+
{
11+
"processors": [
12+
{
13+
"append": {
14+
"if": "ctx?.templating != true",
15+
"field": "foo",
16+
"copy_from": "bar",
17+
"allow_duplicates": false,
18+
"ignore_empty_values": true
19+
}
20+
},
21+
{
22+
"append": {
23+
"if": "ctx?.templating == true",
24+
"field": "foo",
25+
"value": "{{bar}}",
26+
"allow_duplicates": true,
27+
"ignore_empty_values": true
28+
}
29+
},
30+
{
31+
"remove": {
32+
"field": "bar",
33+
"ignore_missing": true
34+
}
35+
},
36+
{
37+
"script": {
38+
"source": "def foo = ctx.foo; ctx.foo_is_null = (foo == null);"
39+
}
40+
}
41+
]
42+
}
43+
- do:
44+
indices.create:
45+
index: "test-some-index"
46+
47+
---
48+
teardown:
49+
- do:
50+
indices.delete:
51+
index: "test-some-index"
52+
ignore_unavailable: true
53+
- do:
54+
ingest.delete_pipeline:
55+
id: "test-pipeline-1"
56+
ignore: 404
57+
58+
---
59+
"scalar values will become a list if necessary":
60+
- do:
61+
index:
62+
index: test-some-index
63+
id: 1
64+
pipeline: test-pipeline-1
65+
body: >
66+
{
67+
"foo": "2",
68+
"bar": "3",
69+
"templating": false
70+
}
71+
72+
- do:
73+
get:
74+
index: test-some-index
75+
id: "1"
76+
- match: { _source.foo: ["2", "3"] }
77+
78+
- do:
79+
index:
80+
index: test-some-index
81+
id: 2
82+
pipeline: test-pipeline-1
83+
body: >
84+
{
85+
"foo": "2",
86+
"bar": "2",
87+
"templating": true
88+
}
89+
90+
- do:
91+
get:
92+
index: test-some-index
93+
id: "2"
94+
- match: { _source.foo: ["2", "2"] }
95+
96+
- do:
97+
index:
98+
index: test-some-index
99+
id: 3
100+
pipeline: test-pipeline-1
101+
body: >
102+
{
103+
"foo": "2",
104+
"bar": "3",
105+
"templating": true
106+
}
107+
108+
- do:
109+
get:
110+
index: test-some-index
111+
id: "3"
112+
- match: { _source.foo: ["2", "3"] }
113+
114+
---
115+
"if a value doesn't change because of ignoring duplicates or empty values, it doesn't become a list":
116+
- do:
117+
index:
118+
index: test-some-index
119+
id: 1
120+
pipeline: test-pipeline-1
121+
body: >
122+
{
123+
"foo": "2",
124+
"bar": "2",
125+
"templating": false
126+
}
127+
128+
- do:
129+
get:
130+
index: test-some-index
131+
id: "1"
132+
- match: { _source.foo: "2" }
133+
134+
- do:
135+
index:
136+
index: test-some-index
137+
id: 2
138+
pipeline: test-pipeline-1
139+
body: >
140+
{
141+
"foo": "2",
142+
"bar": "",
143+
"templating": false
144+
}
145+
146+
- do:
147+
get:
148+
index: test-some-index
149+
id: "2"
150+
- match: { _source.foo: "2" }
151+
152+
- do:
153+
index:
154+
index: test-some-index
155+
id: 3
156+
pipeline: test-pipeline-1
157+
body: >
158+
{
159+
"foo": "2",
160+
"bar": "",
161+
"templating": true
162+
}
163+
164+
- do:
165+
get:
166+
index: test-some-index
167+
id: "3"
168+
- match: { _source.foo: "2" }
169+
170+
---
171+
"an absent value isn't added in the absence of change":
172+
- do:
173+
index:
174+
index: test-some-index
175+
id: 1
176+
pipeline: test-pipeline-1
177+
body: >
178+
{
179+
"bar": "",
180+
"templating": false
181+
}
182+
183+
- do:
184+
get:
185+
index: test-some-index
186+
id: "1"
187+
- match: { _source.foo_is_null: true }
188+
189+
- do:
190+
index:
191+
index: test-some-index
192+
id: 2
193+
pipeline: test-pipeline-1
194+
body: >
195+
{
196+
"bar": "",
197+
"templating": true
198+
}
199+
200+
- do:
201+
get:
202+
index: test-some-index
203+
id: "2"
204+
- match: { _source.foo_is_null: true }
205+
206+
- do:
207+
index:
208+
index: test-some-index
209+
id: 3
210+
pipeline: test-pipeline-1
211+
body: >
212+
{
213+
"templating": false
214+
}
215+
216+
- do:
217+
get:
218+
index: test-some-index
219+
id: "3"
220+
- match: { _source.foo_is_null: true }
221+
222+
- do:
223+
index:
224+
index: test-some-index
225+
id: 4
226+
pipeline: test-pipeline-1
227+
body: >
228+
{
229+
"templating": true
230+
}
231+
232+
- do:
233+
get:
234+
index: test-some-index
235+
id: "4"
236+
- match: { _source.foo_is_null: true }

‎server/src/main/java/org/elasticsearch/ingest/IngestDocument.java‎

Lines changed: 11 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -879,7 +879,9 @@ private void setFieldValue(String path, Object value, boolean append, boolean al
879879
if (object == NOT_FOUND) {
880880
List<Object> list = new ArrayList<>();
881881
appendValues(list, value, allowDuplicates, ignoreEmptyValues);
882-
map.put(leafKey, list);
882+
if (list.isEmpty() == false) {
883+
map.put(leafKey, list);
884+
}
883885
} else {
884886
Object list = appendValues(object, value, allowDuplicates, ignoreEmptyValues);
885887
if (list != object) {
@@ -897,7 +899,9 @@ private void setFieldValue(String path, Object value, boolean append, boolean al
897899
if (object == NOT_FOUND) {
898900
List<Object> list = new ArrayList<>();
899901
appendValues(list, value, allowDuplicates, ignoreEmptyValues);
900-
map.put(leafKey, list);
902+
if (list.isEmpty() == false) {
903+
map.put(leafKey, list);
904+
}
901905
} else {
902906
Object list = appendValues(object, value, allowDuplicates, ignoreEmptyValues);
903907
if (list != object) {
@@ -949,49 +953,24 @@ private static Object appendValues(Object maybeList, Object value, boolean allow
949953
list = new ArrayList<>();
950954
list.add(maybeList);
951955
}
952-
if (allowDuplicates) {
953-
innerAppendValues(list, value, ignoreEmptyValues);
954-
return list;
955-
} else {
956-
// if no values were appended due to duplication, return the original object so the ingest document remains unmodified
957-
return innerAppendValuesWithoutDuplicates(list, value, ignoreEmptyValues) ? list : maybeList;
958-
}
959-
}
960-
961-
// helper method for use in appendValues above, please do not call this directly except from that method
962-
private static void innerAppendValues(List<Object> list, Object value, boolean ignoreEmptyValues) {
963-
if (value instanceof List<?> l) {
964-
if (ignoreEmptyValues) {
965-
l.forEach((v) -> {
966-
if (valueNotEmpty(v)) {
967-
list.add(v);
968-
}
969-
});
970-
} else {
971-
list.addAll(l);
972-
}
973-
} else if (ignoreEmptyValues == false || valueNotEmpty(value)) {
974-
list.add(value);
975-
}
976-
}
977956

978-
// helper method for use in appendValues above, please do not call this directly except from that method
979-
private static boolean innerAppendValuesWithoutDuplicates(List<Object> list, Object value, boolean ignoreEmptyValues) {
980957
boolean valuesWereAppended = false;
981958
if (value instanceof List<?> valueList) {
982959
for (Object val : valueList) {
983-
if (list.contains(val) == false && (ignoreEmptyValues == false || valueNotEmpty(val))) {
960+
if ((allowDuplicates || list.contains(val) == false) && (ignoreEmptyValues == false || valueNotEmpty(val))) {
984961
list.add(val);
985962
valuesWereAppended = true;
986963
}
987964
}
988965
} else {
989-
if (list.contains(value) == false && (ignoreEmptyValues == false || valueNotEmpty(value))) {
966+
if ((allowDuplicates || list.contains(value) == false) && (ignoreEmptyValues == false || valueNotEmpty(value))) {
990967
list.add(value);
991968
valuesWereAppended = true;
992969
}
993970
}
994-
return valuesWereAppended;
971+
972+
// if no values were appended due to duplication/empties, return the original object so the ingest document remains unmodified
973+
return valuesWereAppended ? list : maybeList;
995974
}
996975

997976
private static boolean valueNotEmpty(Object value) {

0 commit comments

Comments
 (0)