Skip building plugins moved to modules#1624
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
dliappis
left a comment
There was a problem hiding this comment.
Works great. Do you think we should add a note in the migrate.rst doc documenting this (and the change in https://github.com/elastic/rally-teams/pull/76/files)?
|
I also hit a weird issue when specifying a build revision as a SHA: If I use a revision with a timestamp like the reproductions you provided in the description it works fine (using either (Note that I have checked out elastic/rally-teams#76 in a sibling director and referenced with However, when I specify the same revision using a commit SHA (again with build or install), things start to fail: |
This comment was marked as outdated.
This comment was marked as outdated.
Ah, I missed that spot. Fixed in 010e08b. |
Good suggestion - addressed in 10ad4fa |
dliappis
left a comment
There was a problem hiding this comment.
Works great. Left a few small comments, but no need for another review cycle.
esrally/mechanic/supplier.py
Outdated
| if plugin.moved_to_module: | ||
| # TODO: https://github.com/elastic/rally/issues/1622 | ||
| continue | ||
|
|
There was a problem hiding this comment.
I think this is a pass here, rather than a continue? (I mean there is no loop)
also a nit: shouldn't the line below be converted to an elif?
There was a problem hiding this comment.
I think this is a pass here, rather than a continue? (I mean there is no loop)
I think continue is the correct choice here because it is inside a for loop.
also a nit: shouldn't the line below be converted to an elif?
Addressed in f6d36e0
There was a problem hiding this comment.
I think
continueis the correct choice here because it is inside aforloop.
Ah ofc missed that 👍
In elastic#1624 we've ensured that plugins that have turned into modules are now skipped from the mandatory plugin check. However, we've also not passed any plugin parameters to the provisioner and this made post installation hooks fail, if they expected parameters to be passed on the command line via `--plugin-params` (e.g. `repository-gcs`). With this commit we make sure that `--plugin-params` is still honored even in these cases. Relates elastic#1624
In #1624 we've ensured that plugins that have turned into modules are now skipped from the mandatory plugin check. However, we've also not passed any plugin parameters to the provisioner and this made post installation hooks fail, if they expected parameters to be passed on the command line via `--plugin-params` (e.g. `repository-gcs`). With this commit we make sure that `--plugin-params` is still honored even in these cases. Relates #1624
Since 8.0, we've converted the
repository-azure,repository-gcsandrepository-s3plugins into Elasticsearch modules, so that they are always included. Whilst adding or removing these plugins still succeeds, it is now a no-op.However, if you try and build these plugins with something like:
It will fail, with this error in Rally's
~/.rally/logs/build.log:For now we still need to retain the existing post-install hooks (see elastic/rally-teams#76), so this is a first step in gracefully skipping any build steps.
Must be merged with:
Relates: