{"action":"created","issue":{"url":"https://api.github.com/repos/markfisher/spring-cloud-deployer/issues/2","repository_url":"https://api.github.com/repos/markfisher/spring-cloud-deployer","labels_url":"https://api.github.com/repos/markfisher/spring-cloud-deployer/issues/2/labels{/name}","comments_url":"https://api.github.com/repos/markfisher/spring-cloud-deployer/issues/2/comments","events_url":"https://api.github.com/repos/markfisher/spring-cloud-deployer/issues/2/events","html_url":"https://github.com/markfisher/spring-cloud-deployer/pull/2","id":134274490,"number":2,"title":"SPI overhaul","user":{"login":"jvalkeal","id":50398,"avatar_url":"https://avatars.githubusercontent.com/u/50398?v=3","gravatar_id":"","url":"https://api.github.com/users/jvalkeal","html_url":"https://github.com/jvalkeal","followers_url":"https://api.github.com/users/jvalkeal/followers","following_url":"https://api.github.com/users/jvalkeal/following{/other_user}","gists_url":"https://api.github.com/users/jvalkeal/gists{/gist_id}","starred_url":"https://api.github.com/users/jvalkeal/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jvalkeal/subscriptions","organizations_url":"https://api.github.com/users/jvalkeal/orgs","repos_url":"https://api.github.com/users/jvalkeal/repos","events_url":"https://api.github.com/users/jvalkeal/events{/privacy}","received_events_url":"https://api.github.com/users/jvalkeal/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":0,"created_at":"2016-02-17T13:05:44Z","updated_at":"2016-02-18T17:20:29Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/markfisher/spring-cloud-deployer/pulls/2","html_url":"https://github.com/markfisher/spring-cloud-deployer/pull/2","diff_url":"https://github.com/markfisher/spring-cloud-deployer/pull/2.diff","patch_url":"https://github.com/markfisher/spring-cloud-deployer/pull/2.patch"},"body":"Overhaul proposal\r\n\r\nI moved most of a stuff from core to spi as it feels a bit wrong for spi package not to keep all its dependant classes. Also changed core to depend on spi while it used to be other way around. Think spi package should be self sustained.\r\n\r\nRemoved AppDeployer.status() as it is impossible to implement and actually not even used in current ModuleDeployer.\r\n\r\n`AppDeploymentId` is now a first class citizen and a central concept between deployer and spi user.\r\n- `AppDeploymentId` now has a `Map<String, String> getProperties()` which deployer can populate order to make un-deploy and status requests more clever.\r\n- Removed link and methods between `AppDefinition` and `AppDeploymentId` as id is created by deployer and definition cannot be used as is to create a fully qualified id to un-deploy or get status.\r\n\r\nPolished javadocs to understand relationships better.\r\n\r\nRemoved count() from `AppDeploymentRequest` as its a bit awkard to keep on a class level.\r\n- Tasks surely would not use it as is.\r\n- `count` as a non-prefixed property would anyway cause trouble at some point as it's way too generic name.\r\n- Think we can keep `count` around as a reserved property name.\r\n- I'm sure we will add more generic properties in a future so I think it's simpler model this way.\r\n\r\nAdmin/Server would need to implement a new Repository interface keeping track of all deployed apps.\r\n- We don't want to implement this behaviour within a deployer as it will just introduce more complexity and deployers are allready rather complex components.\r\n- Also we want to have a single point where this is implemented as otherwise it'd be copy/pasting stuff into different deployers and that eventually leads to broken stuff.\r\n- Tasks will have a bit more complex orchestration around app as there is a separate repos for keeping track of an actual job executions and it also need to query if the actual app running a job is deployed.\r\n\r\nWhile I like an idea of using `ArtifactMetadata` as a marker interface allowing it to be transparent in an `ArtifactResolver` interface, I see some trouble of its use in an `AppDeployer` generic type `A`. Shared controller classes in a server core are the ones using `AppDeployer` api's and looks like type `A` as `MavenCoordinates` would always be hardcoded. If we want to have a different metadata type other than `MavenCoordinates`, how would server core use those? Looks like we'd need to create beans as `mavenStreamAppDeployer` and `mavenTaskAppDeployer` if we some day want to have `ivyStreamAppDeployer` and `ivyTaskAppDeployer`. Hope you get the point I'm chasing here as I don't fully understand the metadata's use concept from server core point of view.\r\n"},"comment":{"url":"https://api.github.com/repos/markfisher/spring-cloud-deployer/issues/comments/185823120","html_url":"https://github.com/markfisher/spring-cloud-deployer/pull/2#issuecomment-185823120","issue_url":"https://api.github.com/repos/markfisher/spring-cloud-deployer/issues/2","id":185823120,"user":{"login":"markfisher","id":140784,"avatar_url":"https://avatars.githubusercontent.com/u/140784?v=3","gravatar_id":"","url":"https://api.github.com/users/markfisher","html_url":"https://github.com/markfisher","followers_url":"https://api.github.com/users/markfisher/followers","following_url":"https://api.github.com/users/markfisher/following{/other_user}","gists_url":"https://api.github.com/users/markfisher/gists{/gist_id}","starred_url":"https://api.github.com/users/markfisher/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/markfisher/subscriptions","organizations_url":"https://api.github.com/users/markfisher/orgs","repos_url":"https://api.github.com/users/markfisher/repos","events_url":"https://api.github.com/users/markfisher/events{/privacy}","received_events_url":"https://api.github.com/users/markfisher/received_events","type":"User","site_admin":false},"created_at":"2016-02-18T17:20:29Z","updated_at":"2016-02-18T17:20:29Z","body":"Regarding \"how would server core use\" `ArtifactMetadata` question... we might want to throw out the marker interface and go with a `String` instead. How that is interpreted (Maven coordinates, S3 bucket, Docker registry path) would be a responsibility of the `ArtifactResolver` in use. Of course that means there is an implicit coupling between the resolver used within the deployer and the requests that are sent to that deployer... not sure how to avoid that. Perhaps we need to extend Spring's `Resource` abstraction instead. That would even create the possibility that one deployer impl could handle multiple types of artifact... perhaps the resolvers could be in a registry based on resource type (s3, docker, maven)."}}