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

Allow passing branch name to bootstrap.sh #964

Merged
merged 3 commits into from
Apr 5, 2016

Conversation

wp-davisona
Copy link
Contributor

This ensures that it is possible to checkout a tag or branch and install from that tag using the same versioned files, for reliable repeatable builds.

e.g.

bootstrap.sh
will still install from the base url
https://raw.githubusercontent.com/getredash/redash/master/setup/ubuntu/files/
but

git checkout v0.9.2.b1536
bootstrap.sh v0.9.2.b1536

will install from the base url:
https://raw.githubusercontent.com/getredash/redash/v0.9.2.b1536/setup/ubuntu/files/postgres_apt.sh

@arikfr
Copy link
Member

arikfr commented Mar 31, 2016

I'm not sure it's worth the hassle. The files it pulls are mainly configuration, which doesn't change much between versions.

Also it's confusing, as I would expect it to install the version specified, which it won't do. The installed version is controlled via the REDASH_VERSION env variable. So to really install b1536 you will need to type

REDASH_VERSION=v0.9.2.b1536 bootstrap.sh v0.9.2.b1536

If someone needs a reliable environment for an older version, I recommend using the relevant Docker image.

@wp-davisona
Copy link
Contributor Author

We've had a couple of occasions where using a static base path has broken our builds. The first was when this path pointed at a docker branch which was deleted and then again when a broken nginx config was committed to master.

This doesn't break any existing behaviour as it still defaults to master and adds flexibility for those who would like to use configurations and scripts from a previously known working version or an experimental branch etc.

I've submitted this as a nice to have, but of course there may be some better method to this.

@arikfr
Copy link
Member

arikfr commented Mar 31, 2016

Fair enough. How about then we use this parameter for the tarball path as well (when it's not master)?

@wp-davisona
Copy link
Contributor Author

Hmm, maybe this would be better as an environment variable too, rather than a command line parameter. Keep it consistent.

Unfortunately, I'm not at a computer till next week, so I'll revisit this then.

@arikfr
Copy link
Member

arikfr commented Apr 4, 2016

Env var sounds good to me, let's name it BRANCH and update the references in code to branch as well, to make sure people aren't confused.

Andrew Davison added 2 commits April 4, 2016 15:37
…REDASH_VERSION.

Move REDASH_VERSION var setup to top of file where it is more visible and easier to update.
@wp-davisona
Copy link
Contributor Author

Ok, I went with REDASH_BRANCH instead of just BRANCH, to be consistent with REDASH_VERSION (but can go with BRANCH if you prefer)

This means a previous release (well, from the point this is merged onwards) can be installed reliably by running:

git checkout v0.9.2.b1536
env REDASH_BRANCH=v0.9.2.b1536 REDASH_VERSION=0.9.2.b1536 ./bootstrap.sh

Note that Branch includes the "v" while Version omits it. It would be possible to merge these variables as one, but then that would remove the flexibility of being able to specify an experimental feature branch based of a specific or current version etc.

Of course running bootstrap.sh on its own without any branch or version still behaves exactly as before.

I also moved the REDASH_VERSION declaration to the top of the bootstrap files as I noticed it defaults to a specific version that needs to be updated whenever there's a new release and this hadn't been done. With it up there at the top of the file it's more noticeable to anyone casually glancing through looking for reasons why they had an unexpected outcome to an installation.

Does this seem cleaner and easier to follow?

@arikfr arikfr changed the title Allow passing a version to bootstrap.sh as an optional cli parameter Allow passing branch name to bootstrap.sh Apr 5, 2016
@arikfr
Copy link
Member

arikfr commented Apr 5, 2016

Looking good. Thanks!

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

Successfully merging this pull request may close these issues.

2 participants