Skip to content

Commit 5c1dbc9

Browse files
authored
Merge branch 'main' into execute-watch
2 parents 16d577d + 07be12f commit 5c1dbc9

File tree

49 files changed

+335
-201
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+335
-201
lines changed

.github/validate-pr/index.js

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -141,12 +141,10 @@ async function run() {
141141
}
142142
comment += `\nYou can validate ${table.length === 1 ? 'this' : 'these'} API${table.length === 1 ? '' : 's'} yourself by using the ${tick}make validate${tick} target.\n`
143143

144-
await octokit.rest.issues.createComment({
145-
owner: 'elastic',
146-
repo: 'elasticsearch-specification',
147-
issue_number: context.payload.pull_request.number,
148-
body: comment
149-
})
144+
core.setOutput('has_results', 'true')
145+
core.setOutput('comment_body', comment)
146+
} else {
147+
core.setOutput('has_results', 'false')
150148
}
151149

152150
core.info('Done!')

.github/workflows/validate-pr.yml

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -60,18 +60,26 @@ jobs:
6060
fi
6161
node scripts/upload-recording/download.js --branch $branch --git
6262
node scripts/clone-elasticsearch/index.js --branch $branch
63-
env:
64-
GCS_CREDENTIALS: ${{ secrets.GCS_CREDENTIALS }}
65-
66-
- name: Remove previous comment
67-
uses: maheshrayas/action-pr-comment-delete@v1
68-
with:
69-
github_token: ${{ secrets.GITHUB_TOKEN }}
70-
org: elastic
71-
repo: elasticsearch-specification
72-
user: github-actions[bot]
73-
issue: ${{ github.event.number }}
7463
7564
- name: Run validation
65+
id: validation
7666
working-directory: ./elasticsearch-specification
7767
run: node .github/validate-pr --token ${{ secrets.GITHUB_TOKEN }}
68+
69+
- name: Find existing comment
70+
uses: peter-evans/find-comment@3eae4d37986fb5a8592848f6a574fdf654e61f9e # v3.1.0
71+
id: find-comment
72+
with:
73+
issue-number: ${{ github.event.pull_request.number }}
74+
comment-author: 'github-actions[bot]'
75+
body-includes: 'Following you can find the validation results'
76+
77+
- name: Create or update comment
78+
if: steps.validation.outputs.has_results == 'true'
79+
uses: peter-evans/create-or-update-comment@71345be0265236311c031f5c7866368bd1eff043 # v4.0.0
80+
with:
81+
token: ${{ secrets.GITHUB_TOKEN }}
82+
comment-id: ${{ steps.find-comment.outputs.comment-id }}
83+
issue-number: ${{ github.event.pull_request.number }}
84+
body: ${{ steps.validation.outputs.comment_body }}
85+
edit-mode: replace

api-design-guidelines/data-modelling.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ Or better yet, to completely avoid using dynamic keys the user-defined value can
150150

151151
### Model object variants in a consumable way
152152

153-
Sometimes an API accepts an object but the keys are determined by what "kind/variant" of the object is intended. An example of this is aggregations, queries, and pipeline steps. There are two ways the Elasticsearch API handles this situation. The first method is using an **internal variant type** property like "type" with analyzers:
153+
Sometimes an API accepts an object but the keys are determined by what "kind/variant" of the object is intended. An example of this is aggregations, queries, and pipeline steps. There are two ways the Elasticsearch API handles this situation. The first method is using **internal tagging** with property like "type" with analyzers:
154154

155155
```yaml
156156
{
@@ -159,7 +159,7 @@ Sometimes an API accepts an object but the keys are determined by what "kind/var
159159
}
160160
```
161161

162-
The second is using **external variants** where the inner object is wrapped with an object with a single key containing the kind of the inner object. This example changes the analyzer from above to use an external variant:
162+
The second is using **external tagging** where the inner object is wrapped with an object with a single key containing the kind of the inner object. This example changes the analyzer from above to use an external variant:
163163

164164
```yaml
165165
{
@@ -169,7 +169,7 @@ The second is using **external variants** where the inner object is wrapped with
169169
}
170170
```
171171

172-
When choosing between these two possibilities **favor using external variants** as it removes the requirement to buffer key-value pairs until the internal variant property is found. Using external variants also improves traversability of the API (ie auto-complete) as properties can be anticipated without waiting for the discriminant property.
172+
Internal tagging is a common way to identify variants and should usually be preferred. However, it may have performance implications when used to deserialize large objects in strongly-typed languages. In that case, external tagging should be considered instead, as it removes the requirement to buffer key-value pairs until the internal variant property is found. Using external tagging also improves traversability of the API (ie auto-complete) as properties can be anticipated without waiting for the discriminant property.
173173

174174
## Model enumerations in a portable way
175175

api-design-guidelines/requests-responses.md

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -97,34 +97,6 @@ It's best to use the body for large or high cardinality variable-length request
9797
- Large objects (Query DSL, aggregations, machine learning models)
9898
- High cardinality variable length values (list of fields, indices, shards, document IDs)
9999

100-
## Adhere to the robustness principle
101-
102-
The robustness principle states:
103-
104-
> "be conservative in what you do, be liberal in what you accept from others"
105-
106-
This principle can apply in a number of ways, but in particular it can determine the design of API requests and responses. An API request may be built flexibly so as to allow both verbose detailed payloads and terse convenience payloads. The corresponding response, however, should always be structured predictably, and not depend on a particular syntax used within the request.
107-
108-
As an example, consider an API that looks up entities based on their ID. The request may be structured to allow either a single ID or a collection of IDs to be passed in. However, the API response should always return a collection of entities, even if that collection only contains one entity.
109-
110-
This principle allows for simpler consumer code that neither has to remember state between request and response, nor needs to "sniff" the output to determine its structure. If multiple variants of the response are truly desired, this may suggest that multiple API endpoints should be introduced, for example called `get_single_entity` and `get_multiple_entities`.
111-
112-
An example of this is the datafeeds API which accepts either a string or list of strings for indices but always returns a list of strings:
113-
114-
```yaml
115-
PUT /_ml/datafeeds/feed-id
116-
{
117-
"indices": "index-name", // Input is a string.
118-
...
119-
}
120-
121-
GET /_ml/datafeeds/feed-id
122-
{
123-
"indices": ["index-name"], // Always returns a list of strings.
124-
...
125-
}
126-
```
127-
128100
## Consider how client functions would wrap the API endpoint
129101

130102
It is common within client-side architecture to provide a one-to-one mapping between API endpoint and client language function. This simplifies implementation and documentation, provides good developer experience, and makes tracking of endpoint usage straightforward.

compiler/src/model/utils.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1094,7 +1094,7 @@ export function parseVariantsTag (jsDoc: JSDoc[]): model.Variants | undefined {
10941094
const nonExhaustive = (typeof tags.non_exhaustive === 'string') ? true : undefined
10951095

10961096
const [type, ...values] = tags.variants.split(' ')
1097-
if (type === 'external') {
1097+
if (type === 'typed_keys_quirk') {
10981098
return {
10991099
kind: 'external_tag',
11001100
nonExhaustive: nonExhaustive

docs/modeling-guide.md

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,9 @@ type FooOrBar = Foo | Bar
332332
333333
An example of internal variants are the type mapping properties.
334334
335-
#### External
335+
#### typed_keys_quirk
336+
337+
**Note**: this feature exists because of some early Elasticsearch APIs where tagging was forgotten, and added after the fact using this quirk to avoid breaking compatibility. **It should not be used for new APIs.**
336338
337339
The key that defines the variant is external to the definition, like in the
338340
case of aggregations in responses or suggesters.
@@ -343,15 +345,15 @@ name in the definition itself.
343345
The syntax is:
344346
345347
```ts
346-
/** @variants external */
348+
/** @variants typed_keys_quirk */
347349

348350
/** @variant name='<field-name>' */
349351
```
350352

351353
For example:
352354

353355
```ts
354-
/** @variants external */
356+
/** @variants typed_keys_quirk */
355357
type FooAlias = Faz | Bar
356358

357359
/** @variant name='faz' */
@@ -369,7 +371,7 @@ In the example above, `FooAlias` will look like this:
369371

370372
```json
371373
{
372-
"faz": {
374+
"name#faz": {
373375
"prop": "hello world"
374376
}
375377
}
@@ -379,7 +381,7 @@ or:
379381

380382
```json
381383
{
382-
"bar": {
384+
"name#bar": {
383385
"prop": true
384386
}
385387
}

docs/overlays/elasticsearch-openapi-overlays.yaml

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,4 +65,12 @@ actions:
6565
- target: "$.components['schemas']['_types.aggregations.RandomSamplerAggregation']"
6666
description: Add x-model
6767
update:
68-
x-model: true
68+
x-model: true
69+
# Remove long lists of enum values
70+
- target: "$.components['schemas']['cat._types.CatNodeColumn'].anyOf"
71+
description: Remove anyOf from cat nodes
72+
remove: true
73+
- target: "$.components['schemas']['cat._types.CatNodeColumn']"
74+
description: Add basic string data type for cat node columns
75+
update:
76+
type: string

docs/overlays/elasticsearch-shared-overlays.yaml

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1079,5 +1079,17 @@ actions:
10791079
# x-model: true
10801080
# Remove long lists of enum values
10811081
- target: "$.components['schemas']['cat._types.CatAnomalyDetectorColumn'].enum"
1082-
description: Remove enum array
1082+
description: Remove enum array from cat detectors
1083+
remove: true
1084+
- target: "$.components['schemas']['cat._types.CatDfaColumn'].enum"
1085+
description: Remove enum array from cat data frame analytics
1086+
remove: true
1087+
- target: "$.components['schemas']['cat._types.CatDatafeedColumn'].enum"
1088+
description: Remove enum array from cat datafeeds
1089+
remove: true
1090+
- target: "$.components['schemas']['cat._types.CatTrainedModelsColumn'].enum"
1091+
description: Remove enum array from cat trained models
1092+
remove: true
1093+
- target: "$.components['schemas']['cat._types.CatTransformColumn'].enum"
1094+
description: Remove enum array from cat transforms
10831095
remove: true

0 commit comments

Comments
 (0)