search

strongloop / strong-pm

deployer for node applications
http://strong-pm.io
Other
1k stars 71 forks source link

server: support old-style deploy to / #192

Closed sam-github closed 9 years ago

sam-github commented 9 years ago

Current pm uses deploy to /api/Service//deploy, but old-style deploys to /, and arc needs to work with both old (pre-multiapp) and new process managers.

connected to strongloop-internal/scrum-nodeops#511

sam-github commented 9 years ago

This doesn't deal with creating a service if necessary. I'll update.

rmg commented 9 years ago

Looks like it should be pretty easy to add a test case for this in the existing deploy tests simply by overriding the URL.

sam-github commented 9 years ago

so far, the tests don't allow the URL to be overridden, they just call sl-deploy, it constructs the url, and I can't have two versions of sl-deploy. but I'm still looking.

rmg commented 9 years ago

If only the tests were shelling out to git instead :-P (actually, the docker and Vagrant tests still are)

sam-github commented 9 years ago

Better to unit test anyway, PTAL.

rmg commented 9 years ago

Your test doesn't ensure that the git receiver actually functions at the alternate URL.. but I'm assuming you have tested that manually.

LGTM.

sam-github commented 9 years ago

Yes, the git receiver has no dependency on the URL. I'm trying to avoid integration tests: though the git receiver is integration tested by the pmctl commands, and this code as well as the integration tests are just two different ways to hit ServerService.deploy().

rmg commented 9 years ago

It's more about the multiple-request (to different URLs) nature of the git protocol and making sure the subsequent requests all get handled appropriately. This is the part that we had problems with before with git, which is the cause for my paranoia.

rmg commented 9 years ago

LGTM if this is the way you want to go.

rmg commented 9 years ago

LGTM.

sam-github commented 9 years ago

I'll merge after https://github.com/strongloop/strong-pm/pull/195