Allows storm-deploy more flexibility - checkout branches and SHA1s #45

Merged
merged 2 commits into from Oct 23, 2013

Conversation

Projects
None yet
4 participants
@lorcan
Contributor

lorcan commented Oct 3, 2013

The storm --release model is very unclear and not all that useful. Currently there is a choice of passing a no release, in which case you get a version of storm based on the latest commit from master or you get to pass a release which gets you the latest commit from that branch, e.g.,

lein deploy-storm --start --name mycluster --release 0.9.0

gets you storm 0.9.0wip16 (roughly, actually it gets you commit (d12c33543c9b2e8c8e35908f8672fa06edd18c2e, which is between 0.9.0-wip16 and 0.9.0-wip17)

and

lein deploy-storm --start --name mycluster

gets you the latest commit from master. It's not possible to get anything inbetween.

This commit drops the --release argument in favour of a --branch argument (equivalent to --release) and a --commit argument, which takes a SHA1. If a commit is passed then we attempt to checkout that SHA1 into a new branch and use that. This gives us much more flexibility, e.g., to checkout storm 0.9.0-rc2 you would run

    lein deploy-storm --start --name mycluster --branch master --commit 32098d5b2694434ea43d430a4703fbe51bab268f 

If you want the latest version from a specific branch (say 0.8.3) you can execute

    lein deploy-storm --start --name mycluster --branch 0.8.3

and if you want the bleeding edge you can still execute this get the latest commit from master

    lein deploy-storm --start --name mycluster

The diff on this commit is bigger than it might have been because i deliberately changed release to branch wherever possible. I think now that storm has both releases (since 0.9.9-rc1) and branches we should make the code closer to what's actually happening (i.e., checking out branches rather than releases).

I'd love to get feedback on this. I've played around with it today but I wouldn't go so far to say it's perfect. I'd like to see if anyone else has had success with it before pulling it into nathanmarz/storm-deploy proper.

This includes @xpe's pull request nathanmarz/storm-deploy#40 and @lorcan's pull requests nathanmarz/storm-deploy#43 and nathanmarz/storm-deploy#44

Likely fix for #39.
I think that the upgrade from jClouds 1.4.2 to 1.5.1 in the previous
commit (9d6e47b) made a change in how `.createSecurityGroupInRegion`
works (see security.clj).
@msegel

This comment has been minimized.

Show comment
Hide comment
@msegel

msegel Oct 1, 2013

This fix works!
Thx.

msegel commented on 34b0d2a Oct 1, 2013

This fix works!
Thx.

This comment has been minimized.

Show comment
Hide comment
@lorcan

lorcan Oct 1, 2013

Owner

Cool, you should comment on the main repo to add weight to getting this pulled into storm-deploy proper :-)

Owner

lorcan replied Oct 1, 2013

Cool, you should comment on the main repo to add weight to getting this pulled into storm-deploy proper :-)

Allows storm-deploy to select both a storm branch and a SHA1 commit
The storm `--release` model is very unclear and not all that useful.
Currently there is a choice of passing a no release, in which case you
get a version of storm based on the latest commit from master or you
get to pass a release which gets you the latest commit from that
branch, e.g.,

    lein deploy-storm --start --name mycluster --release 0.9.0

gets you storm 0.9.0wip16 (roughly, actually it gets you commit
(d12c33543c9b2e8c8e35908f8672fa06edd18c2e, which is between 0.9.0-wip16
and 0.9.0-wip17)

and

    lein deploy-storm --start --name mycluster

gets you the latest commit from master. It's not possible to get
anything inbetween.

This commit drops the `--release` argument in favour of a `--branch`
argument (equivalent to `--release`) and a `--commit` argument, which
takes a SHA1. If a commit is passed then we attempt to checkout that
SHA1 into a new branch and use that. This gives us much more
flexibility, e.g., to checkout storm 0.9.0-rc2 you would run

        lein deploy-storm --start --name mycluster --branch master
--commit 32098d5b2694434ea43d430a4703fbe51bab268f

If you want the latest version from a specific branch (say 0.8.3) you
can execute

        lein deploy-storm --start --name mycluster --branch 0.8.3

and if you want the bleeding edge you can still execute this get the
latest commit from master

        lein deploy-storm --start --name mycluster

The diff on this commit is bigger than it might have been because i
deliberately changed `release` to `branch` wherever possible. I think
now that storm has both releases (since 0.9.9-rc1) and branches we
should make the code closer to what's actually happening (i.e.,
checking out branches rather than releases).

tbatchelli added a commit that referenced this pull request Oct 23, 2013

Merge pull request #45 from lorcan/checkoutcommit
Allows storm-deploy more flexibility - checkout branches and SHA1s

@tbatchelli tbatchelli merged commit 6cf865a into nathanmarz:master Oct 23, 2013

@lorcan lorcan deleted the lorcan:checkoutcommit branch Oct 24, 2013

@msegel

This comment has been minimized.

Show comment
Hide comment
@msegel

msegel Oct 31, 2013

Perhaps it would be good to document how you can get the different SHA1 commits?

msegel commented Oct 31, 2013

Perhaps it would be good to document how you can get the different SHA1 commits?

@lorcan

This comment has been minimized.

Show comment
Hide comment
@lorcan

lorcan Nov 1, 2013

Contributor

What do you have in mind? The SHA1 commits are listed on each release and indeed on every code listing. I think there is a bigger issue here and that is that most users will never want to checkout by SHA1 commits - they would likely prefer to use tagged releases (which are a recent addition to storm) - see #46.

Contributor

lorcan commented Nov 1, 2013

What do you have in mind? The SHA1 commits are listed on each release and indeed on every code listing. I think there is a bigger issue here and that is that most users will never want to checkout by SHA1 commits - they would likely prefer to use tagged releases (which are a recent addition to storm) - see #46.

@msegel

This comment has been minimized.

Show comment
Hide comment
@msegel

msegel Nov 1, 2013

Well the sha1 hash isn't really human readable or memorable.
That means that I either have to store it somewhere or alwasy get online to the main branch and find the hash that matches the tag I want.

I mean I can remember 0.9.0-wipX or 0.9.0-rc2 but a 128bit string of randomness?
I'm sorry, I'm old and I can barely remember what I had for dinner last night... ;-)

msegel commented Nov 1, 2013

Well the sha1 hash isn't really human readable or memorable.
That means that I either have to store it somewhere or alwasy get online to the main branch and find the hash that matches the tag I want.

I mean I can remember 0.9.0-wipX or 0.9.0-rc2 but a 128bit string of randomness?
I'm sorry, I'm old and I can barely remember what I had for dinner last night... ;-)

@lorcan

This comment has been minimized.

Show comment
Hide comment
@lorcan

lorcan Nov 1, 2013

Contributor

Completely agree, no human should have to deal with SHA1s :-) Check out the updated documentation. Tagged releases are discussed, you can checkout a tagged release of storm as follows:

lein deploy-storm --start --name mycluster --branch master --commit 0.9.0-rc2

Users should be allowed to check out by SHA1 but generally I doubt many people will use that as the preferred option - more likely people will want tagged releases, or latest commits on particular branches...

Contributor

lorcan commented Nov 1, 2013

Completely agree, no human should have to deal with SHA1s :-) Check out the updated documentation. Tagged releases are discussed, you can checkout a tagged release of storm as follows:

lein deploy-storm --start --name mycluster --branch master --commit 0.9.0-rc2

Users should be allowed to check out by SHA1 but generally I doubt many people will use that as the preferred option - more likely people will want tagged releases, or latest commits on particular branches...

@msegel

This comment has been minimized.

Show comment
Hide comment
@msegel

msegel Nov 1, 2013

Ah cool

That's what is needed!

Thx
On Nov 1, 2013, at 6:48 AM, Lorcan Coyle notifications@github.com wrote:

Completely agree, no human should have to deal with SHA1s :-) Check out the updated documentation. Tagged releases are discussed, you can checkout a tagged release of storm as follows:

lein deploy-storm --start --name mycluster --branch master --commit 0.9.0-rc2
Users should be allowed to check out by SHA1 but generally I doubt many people will use that as the preferred option - more likely people will want tagged releases, or latest commits on particular branches...


Reply to this email directly or view it on GitHub.

msegel commented Nov 1, 2013

Ah cool

That's what is needed!

Thx
On Nov 1, 2013, at 6:48 AM, Lorcan Coyle notifications@github.com wrote:

Completely agree, no human should have to deal with SHA1s :-) Check out the updated documentation. Tagged releases are discussed, you can checkout a tagged release of storm as follows:

lein deploy-storm --start --name mycluster --branch master --commit 0.9.0-rc2
Users should be allowed to check out by SHA1 but generally I doubt many people will use that as the preferred option - more likely people will want tagged releases, or latest commits on particular branches...


Reply to this email directly or view it on GitHub.

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