Can we verify that parameters passed by the user are actually used?

[danielmitterdorfer]

We allow users to pass parameters in various places, e.g. with --track-params. These parameters are then passed to Jinja2 internally which may or may not use those parameters. If the key has a typo, e.g. cliant instead of client the parameter will simply not be used and the user might not notice.

We should evaluate whether it is possible to verify that each track parameter has been successfully consumed by Jinja2 and abort the race with a clear error message if not.

[danielmitterdorfer]

We can probably leverage Jinja's meta API for that. However, one problem seems to be that we use macros in our tracks to include further templates, e.g.

  "operations": [
    {{ rally.collect(parts="operations/*.json") }}
  ]

This means either:

• We do not use these macros anymore.
• We find a way to render the full template as an intermediate step.

[dliappis]

I think b47309f has paved the way for the second option presented above.

[dliappis]

Spoke too soon, I had a first pass at this and seems more convoluted than initially thought.
The meta API requires parsing the entire source. However, j2 include macros are making it difficult to assemble the entire source without rendering it, which is what parse() requires to build the AST.

I took a look at different approaches in loading (e.g. some suggested here: http://codyaray.com/2015/05/auto-load-jinja2-macros) but so far didn't find a non-hacky way to assemble the complete source referenced by macro includes in one source, so that it can get parsed for undeclared variables.

[dliappis]

@danielmitterdorfer I spent a bit of time digging into this again.

Firstly I found some additional evidence in https://stackoverflow.com/a/4294898 that indeed it will be really difficult to "interrupt" the parsing process and only parse includes but skip parsing of {{ track_param_foo | default(100) }} type of j2 variables.

However, I still think the use of rally.collect offers value with large tracks (e.g. eventdata track etc.). Additionally I wouldn't like to break existing users and do major changes to our tracks (on all branches).

Another approach that we've tangentially discussed in the past is, prior to render_template, to have an additional step where we assemble the entire source -- without the use of j2 helpers -- and replace references to {{ rally.collect(parts="operations/*.json") }} with the corresponding files. This lets us assemble the entire source sans the includes which makes the meta API useful again.

I created a PoC here: https://gist.github.com/dliappis/fd1c5ffa2adb74f07bb62f1dc8974aa9

Executing this Python snippet will succeed with no output, but if we change the track param in line 16: https://gist.github.com/dliappis/fd1c5ffa2adb74f07bb62f1dc8974aa9

to ingest-params (simulating a typo) we get:

$ ~/Downloads/foo.py
WARNING: You've declared [ingest-percentage] track-param but this is not overriding ANY of the j2 variables in the track

There's nothing ground breaking in the PoC. It just uses a reasonable regex (line 31) which is lenient with spaces and will match any ref to {{ rally.collect(parts="<file_or_glob>") }} with the actual files.

If this PoC makes sense, I can work on a Rally PR.

Note: My PoC doesn't currently recursively parse rally.collect e.g. in included files, but we can deal with this recursively without problems.

[danielmitterdorfer]

Although this approach (in the PoC) is duplicating what the helper is doing I think it improves the user experience a lot. Hence I think we should move forward with this. Thank you for looking into it!