-
Notifications
You must be signed in to change notification settings - Fork 4
Deploy fat jar to Azure via Travis on tags #39
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with questions.
check_preconditions() { | ||
if [ -z "${tag}" ]; then | ||
log "Build is not a tag, skipping publish" | ||
exit 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's your opinion on creating a fat jar for every commit instead of just tags?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to say I prefer tags, but if we do it automatically, we may have to navigate through a bunch of junk tags in order to promote one for uploading. By the smae token, we may also end up uploading a lot of unnecessary jars. Something else we could do is to have a release branch and we merge to it when we want to upload.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, let's stick with the tags then :) Thanks for your input.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if you considered this, but shouldnt we create a release in GH https://developer.github.com/v3/repos/releases/#create-a-release?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just confirmed that this works. Here is the build for a release that I kicked off via the Github UI.
fi | ||
if [ -z "${blobaccount}" ] || [ -z "${blobkey}" ] || [ -z "${blobcontainer}" ]; then | ||
log "Azure blob connection is not set, unable to publish builds" | ||
exit 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is runs after success, these non-zero exit codes won't block the PR, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct.
--account-key "${blobkey}" \ | ||
--file "${fatjar}" \ | ||
--container "${blobcontainer}" \ | ||
--blob "${blobname}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any chance we could upload to blob storage via openssl
+ curl
so we don't have to install the cli and npm?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately it's non-trivial to talk to Azure via curl (see this StackOverflow post) so I don't think it's worth it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great @c-w . Left a couple comments below.
Thanks!
curl -sL https://deb.nodesource.com/setup_6.x | sudo -E bash - | ||
sudo apt-get update | ||
sudo apt-get install -y -qq nodejs | ||
sudo npm install -g npm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldnt we minimize the build time by installing the azure-cli
apt-get package as opposed to installing node? i.e. sudo apt-get -y update && sudo apt-get install -y azure-cli
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried a number of ways to install the azure-cli (e.g. via apt-get or the v2) and this is the only way that I got to work reliably. The apt-get package installs a really old version of node and npm which breaks some functionality downstream.
check_preconditions() { | ||
if [ -z "${tag}" ]; then | ||
log "Build is not a tag, skipping publish" | ||
exit 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if you considered this, but shouldnt we create a release in GH https://developer.github.com/v3/repos/releases/#create-a-release?
After merging this pull request, whenever we push a tag to Github, Travis will as part of the CI automatically build a fat jar that contains project-fortis-spark and all of its dependencies and then upload that jar to a public Azure Blob storage from where our cluster launching script can pick up the jar and pass it to spark-submit.
Sample run:
This process takes about 5 minutes to run, so we could even consider running it on every Travis run (not just on tags) and name the created jars by their commit hash. This is a trivial one-line change to the
.travis/publish.sh
script so just let me know if you'd like that option.Another thing that we can implement is to upload a second copy of the jar to a name like
fortis-latest.jar
so that the deployment script can always pick up the latest jar without having to be modified. Thoughts?Resolves #31