strongloop / strong-deploy

Deploy nodejs applications
Other
13 stars 3 forks source link

Enable strong-deploy to be used as a module #12

Closed kraman closed 9 years ago

kraman commented 9 years ago

/cc @ritch

sam-github commented 9 years ago

Can we just move all the deployCli() code (getopts, etc.) into bin/sl-deploy.js? Leaving index.js to just be a js module exporting that re-useable deploy()?

Also, I have some doubt's about whether factoring this into a module is this easy. In particular, the deploy depends heavily on the CWD, IIRC, so you might need to take a working directory as an option to deploy, and ensure that all operations done by the deploy can occur when process.cwd() is not the root of the package being deployed. Which we can do later, but it's a big caveat in the current API, isn't it?

kraman commented 9 years ago

This PR might not be needed. @ritch was looking at using lib/git.js and lib/put-file.js directly

sam-github commented 9 years ago

LGTM

rmg commented 9 years ago

How will studio/arc be using the code? Will the front-end tolerate the backend being inaccessible while this code blocks the event loop and therefore all request handlers?

kraman commented 9 years ago

@rmg What part are you afraid would block for extended periods of time? The only sync/blocking code here operates on local git content and should be fairly quick.

kraman commented 9 years ago

/cc @ritch

rmg commented 9 years ago

@kraman not as much of a problem with deploy as with build. For deploy the problem would be pushing a large repo over a slow connection, a connection problem resulting in a timeout, a lengthy git gc --auto, or other similar event. In any blocking case there's no ability to interrupt, abort, or notify the user.

kraman commented 9 years ago

@rmg git push is handled as an exec with as async callback so it should be fine. We are not changing build right now. @ritch has already written code to shell out to it.

rmg commented 9 years ago

So it does. The other 3 git shell outs are synchronous, so I made the mistake of assuming they all were. I'll be quiet now, for at least 15 minutes ;-)

cgole commented 9 years ago

@kraman what do I have to verify in this story?

sam-github commented 9 years ago

@cgole you don't have to verify, its internal, the Arc deploy feature will be what is verified (when complete)

sam-github commented 9 years ago

@kraman did you hand-merge by accident? There is no merge commit, it was just fast-forwarded onto master.

kraman commented 9 years ago

@sam-github Did a rebase + merge. Did we switch to using the Github merge button?

sam-github commented 9 years ago

I'm confused... everything uses github merge (except for strongops :-( ). When do you not use the merge button? All the PRs you merge that I can recall you've done the github way.

rmg commented 9 years ago

I've only used the merge button a few times in the last year. I almost always rebase, push +branch, & merge, which generally results in a fast-forward.

kraman commented 9 years ago

Same here, Ive always been doing rebase + FF merge. Im fine either way, let me know what you two prefer.