-
Notifications
You must be signed in to change notification settings - Fork 313
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 template key requirement from composable template param source #1486
Conversation
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.
I tested current behavior, without this PR, to see what Rally does with a composable index template that is missing a template
object, and contains a composed_of
array with a single component template:
{
"name": "create-template-test",
"operation-type": "create-composable-template",
"template": "metricbeat-template-1",
"body": {
"index_patterns": [
"metricbeat"
],
"composed_of": [
"metricbeat-component-1"
]
}
}
A create-component-template
operation was also added:
{
"name": "create-template-metricbeat-component-1",
"operation-type": "create-component-template",
"template": "metricbeat-component-1",
"body": {
"template": {
"settings": {
"number_of_shards": 1
},
"mappings": {
"_source": {
"enabled": false
}
}
}
}
}
As documented in the 'Note' section, these operations should be executed in order of create-component-template
, then create-composable-template
to ensure any component templates referenced by the composable template exist prior to creating the composable template.
I found Rally to create both the component and composable templates correctly when the challenges are specified in the correct order. However, Rally unexpectedly continues when the composable template is first, followed by the component template. Additionally, there is no sign of the 400
error it should have received when creating a composable template referencing a non-existent component template. Rally should not be allowed to continue in the case where creating the index template (composable) results in a 400
error code:
PUT /_index_template/metricbeat-template-1
{
"error": {
"root_cause": [
{
"type": "invalid_index_template_exception",
"reason": "index_template [metricbeat-template-1] invalid, cause [index template [metricbeat-template-1] specifies component templates [metricbeat-component-1] that do not exist]"
}
],
"type": "invalid_index_template_exception",
"reason": "index_template [metricbeat-template-1] invalid, cause [index template [metricbeat-template-1] specifies component templates [metricbeat-component-1] that do not exist]"
},
"status": 400
}
Line 447 appears to be checking for the template/settings
path to apply settings to the template, and not as a form of validation for the template itself.
This assumption caused valid composable templates to get skipped when using the native
create-composable-template
task.
Could the issue have been somewhere else, like in challenge ordering?
Unfortunately not, i would have received a 400 error from ES if that were
the symptom. The problem likely only applies when referencing templates
from the top-level track element and not as explicit body content, since
the code changed is composing the body parameter from a given template.
…On Mon, May 9, 2022, 5:46 PM Jason Bryan ***@***.***> wrote:
***@***.**** requested changes on this pull request.
I tested current behavior, without this PR, to see what Rally does with a
composable index template that is missing a template object, and contains
a composed_of array with a single component template:
{
"name": "create-template-test",
"operation-type": "create-composable-template",
"template": "metricbeat-template-1",
"body": {
"index_patterns": [
"metricbeat"
],
"composed_of": [
"metricbeat-component-1"
]
}
}
A create-component-template operation was also added:
{
"name": "create-template-metricbeat-component-1",
"operation-type": "create-component-template",
"template": "metricbeat-component-1",
"body": {
"template": {
"settings": {
"number_of_shards": 1
},
"mappings": {
"_source": {
"enabled": false
}
}
}
}
}
As documented
<https://esrally.readthedocs.io/en/latest/track.html?highlight=template#create-composable-template>
in the 'Note' section, these operations should be executed in order of
create-component-template, then create-composable-template to ensure any
component templates referenced by the composable template exist prior to
creating the composable template.
I found Rally to create both the component and composable templates
correctly when the challenges are specified in the correct order. However,
Rally unexpectedly continues when the composable template is first,
followed by the component template. Additionally, there is no sign of the
400 error it should have received when creating a composable template
referencing a non-existent component template. Rally should not be allowed
to continue in the case where creating the index template (composable)
results in a 400 error code:
PUT /_index_template/metricbeat-template-1
{
"error": {
"root_cause": [
{
"type": "invalid_index_template_exception",
"reason": "index_template [metricbeat-template-1] invalid, cause [index template [metricbeat-template-1] specifies component templates [metricbeat-component-1] that do not exist]"
}
],
"type": "invalid_index_template_exception",
"reason": "index_template [metricbeat-template-1] invalid, cause [index template [metricbeat-template-1] specifies component templates [metricbeat-component-1] that do not exist]"
},
"status": 400
}
Line 447
<https://github.com/elastic/rally/blob/609a043223d0fb7e180f40def4d568064078e672/esrally/track/params.py#L447>
appears to be checking for the template/settings path to apply settings
to the template, and not as a form of validation for the template itself.
This assumption caused valid composable templates to get skipped when
using the native create-composable-template task.
Could the issue have been somewhere else, like in challenge ordering?
—
Reply to this email directly, view it on GitHub
<#1486 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABZBUUIC3EFHLQPCVTH4Z7DVJGBTXANCNFSM5VPAXH6A>
.
You are receiving this because you were assigned.Message ID:
***@***.***>
|
closing as insufficient. opened #1487 to track the issue |
With the following track (composable template referring to a non existent component template) I get the 400 error as expected:
Also with this other track (composable created before the component) I get 400 as expected:
|
Note that this change only influences templates provided by the top-level track field |
According to the API,
template
is not a required key in the create request. This assumption caused valid composable templates to get skipped when using the nativecreate-composable-template
task.