Skip to content
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

Parameterize OS name + CPU arch in download URL. #32

Merged
merged 1 commit into from
Oct 2, 2019
Merged

Conversation

drawlerr
Copy link

@drawlerr drawlerr commented Oct 1, 2019

We should use the new template parameters to get the correct version of ES for our platform + arch.

Copy link
Member

@danielmitterdorfer danielmitterdorfer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for raising the PR. The change looks fine to me. However, I have a couple of notes:

  • We should hold back from merging this at least until Rally 1.5.0 is out. This is a breaking change that will only work with Rally 1.4.0 or better and we should give users some time to upgrade.
  • We also need to forward-port this to the master branch. Usually I create PRs against master and backport the change as far as needed but personally I'm ok if we keep this PR as is and forward port it.

@danielmitterdorfer
Copy link
Member

@drawlerr I just noticed that we should also customize the vanilla config:

release_url = https://artifacts.elastic.co/downloads/elasticsearch/elasticsearch-oss-{{VERSION}}-linux-x86_64.tar.gz

Can you please change it there as well?

@drawlerr drawlerr merged commit 760907f into elastic:7 Oct 2, 2019
danielmitterdorfer added a commit that referenced this pull request Oct 2, 2019
@danielmitterdorfer danielmitterdorfer added the blocked This item cannot be finished because of a dependency label Oct 8, 2019
@danielmitterdorfer danielmitterdorfer removed the blocked This item cannot be finished because of a dependency label Jan 9, 2020
@danielmitterdorfer
Copy link
Member

superseded by #38

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants