Inject build_flavor in track templates#1750
Merged
gbanasiak merged 4 commits intoelastic:masterfrom Jul 26, 2023
Merged
Conversation
dliappis
reviewed
Jul 18, 2023
Contributor
dliappis
left a comment
There was a problem hiding this comment.
The approach looks good to me and in tests I did it worked well. Left a very minor comment.
dliappis
reviewed
Jul 18, 2023
ebadyano
reviewed
Jul 25, 2023
| * ``build_flavor``: a global variable that holds build flavor reported by Elasticsearch cluster targetted by Rally. | ||
| * ``days_ago()``: a `filter <http://jinja.pocoo.org/docs/dev/templates/#filters>`_ that you can use for date calculations. | ||
|
|
||
| You can find an example in the ``http_logs`` track:: |
Contributor
There was a problem hiding this comment.
This might be a bit confusing, since the example only talks about now and days_ago. Not sure if we may want to add an example with bulld_flavour as well? and specify that the http_log example is for the other two?..
Contributor
Author
There was a problem hiding this comment.
Right, I haven't thought about this example below. I moved build_flavor variable description lower in 29351b4. I don't have an example that would make sense outside of serverless context, so I'm not adding it.
ebadyano
reviewed
Jul 25, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Here's an attempt at injecting
build_flavorin track templates. It is part of Rally modifications for serverless.The idea is to handle
build_flavoras Jinja2 internal variable, similarly tonow(doc).Internal variables take precedence over user-supplied parameters and are excluded from a list of parameters exposed by a track (see here). As a result, today if a user provides
nowparameter via--track-paramsit is not recognised as a valid parameter even if it is present in a track definition. I found current error slightly misleading so this PR adds a dedicated error message when user-provided parameter collides with any of the variables defined internally.Internal variables are not used today when rendering index settings, index templates, component templates and composable templates. This PR changes that adding
build_flavorinternal variable alone to these as well. That's necessary to support scenarios such as elastic/rally-tracks#406 wherebuild_flavorcould be used in component template definition.With this approach
build_flavorcannot be determined within custom runner or custom parameter source Python code. Their interface simply does not expose the necessary data structures. To workaround this we would need to injectbuild_flavoras an extra operation parameter which I did not find compelling. I don't think it's a blocker becausebuild_flavor-driven customisation can still be achieved at the track definition level.The
build_flavorvalue is taken from Elasticsearch REST API call in an early stage of benchmark preparation flow, i.e.setup()inBenchmarkCoordinator(see #1748 diagram) and saved to configuration here. This call does not happen in case of source builds. If configuration entry is missing,defaultvalue ofbuild_flavoris assumed.This is a breaking change because
build_flavorbecomes reserved, so the plan is toreleasebump up version to2.8.1and2.9.0before merging it.