search

forward3d / cap-ec2

Capify-EC2 rewritten for Capistrano v3
MIT License
181 stars 118 forks source link

Allow deploying multiple projects per instance #17

Closed rsslldnphy closed 10 years ago

rsslldnphy commented 10 years ago

We'd like to be able to deploy multiple apps to the same set of instances so I've put together this change which allows you to set a comma-delimited list of project names as the value of the project_tag.

rsslldnphy commented 10 years ago

I did think about this as it happens, and added a second filter on the returned instances as part of the select block to account for this, by splitting the returned project_tag on commas and asserting that there is an exact match in the returned list.

Unfortunately I don't think that regex is doing exactly what you think it's doing... the bit at the end ((,|$)) works as you would expect it to, and matches either a comma or the end of the line, but the bit at the start (,{0,1}) isn't preceded by (and wouldn't work if preceded by) a ^, so what it is actually saying is "There is either a comma here or there isn't a comma here" - which doesn't actually say anything.

Checking it in Rubular confirms that ,{0,1}cats(,|$) matches cats ,cats and definitelynotcats.

So at the moment there is a risk that you could deploy to the wrong servers as you describe - but I've also changed those regexes to splitting the strings on commas and testing that the returned array include?s the correct value so that should fix that. One thing I haven't done but could do it also trim the strings after splitting on commas to take into account whitespace? I'll add another commit for that unless you have any objections.

andytinycat commented 10 years ago

Oh my, I apologise! I can see you're entirely correct - that regex is functionally useless. I obviously wasn't thinking very clearly when I wrote it!

Please push any commits for fixing up the matching along with this feature and I'll merge it all in, including the whitespace fixes you mention.

Excellent catch.

rsslldnphy commented 10 years ago

Great, thanks! I've pushed up a commit that trims the whitespace before checking (and have tested it with one of our projects).

andytinycat commented 10 years ago

Looks good. I'll test here and cut a new gem in a few minutes.

andytinycat commented 10 years ago

I've cut 0.0.11 with this feature and the fixes. Thanks very much for your contributions - I'm always happy to take PRs for any other features you need or bugs you spot.

rsslldnphy commented 10 years ago

Great, thanks again!