Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

As a service author I can use the abstractions provided by Spring Cloud Deployer as part of a multi-flow reactive pipeline #24

Closed
ojhughes opened this issue May 4, 2018 · 5 comments
Assignees
Labels
Milestone

Comments

@ojhughes
Copy link
Contributor

ojhughes commented May 4, 2018

Currently the Deployer SPI provides non reactive signatures eg String deploy(AppDeploymentRequest request) (see [https://github.com/spring-cloud/spring-cloud-deployer/blob/master/spring-cloud-deployer-spi/src/main/java/org/springframework/cloud/deployer/spi/app/AppDeployer.java]

To use Deployer effectively within a multi-flow reactive pipeline, there would need to be alternative, reactive interfaces provided by the Deployer SPI, eg Mono<String> deploy(AppDeploymentRequest request)

It should be feasible to provide reactive alternatives to the default implementations of the Reactive SPI (Cloud Foundry, Kubernetes, Local)

cc @sabbyanandan

@ojhughes ojhughes changed the title As a developer I can use the abstractions provided by Spring Cloud Deployer with a reactive pipeline As a developer I can use the abstractions provided by Spring Cloud Deployer as part of a multi-flow reactive pipeline May 4, 2018
@ojhughes ojhughes self-assigned this May 9, 2018
@scottfrederick scottfrederick changed the title As a developer I can use the abstractions provided by Spring Cloud Deployer as part of a multi-flow reactive pipeline As a service author I can use the abstractions provided by Spring Cloud Deployer as part of a multi-flow reactive pipeline May 9, 2018
@ojhughes ojhughes added in progress and removed next labels May 14, 2018
@ojhughes
Copy link
Contributor Author

The simplest way to achieve this is introducing a new class CloudFoundryAppDeployerOperations that contains all the functionality needed by both the blocking and non-blocking interface.

@ojhughes
Copy link
Contributor Author

I don't think it is feasible to introduce Reactive versions of the existing S-C-Deployer implementations without the risk of breaking other consumers. This is because a lot of functionality is with an abstract base class

I tried introducing a shared operations class that could be used by both reactive and non-reactive implementations. This quickly started falling apart because a lot of the functionality that would need to be shared via composition exists in the abstract class.

I think it is reasonable to not use Reactor directly to begin with, instead using deployer with an ExecutorService when parallel deployments are required

@ojhughes
Copy link
Contributor Author

Maybe I wrote this off too early..
There was another discussion about this some time age spring-cloud/spring-cloud-deployer#19

I think my fears about changing the abstract base class could be rectified by removing it completely and replacing with delegate. Would be good to discuss @royclarkson @scottfrederick

@ojhughes ojhughes removed the accepted label May 31, 2018
@ojhughes
Copy link
Contributor Author

@royclarkson
Copy link
Member

We can certainly improve that code if we are returning reactive types. IIUC, blocking there was because they wanted the exception thrown. If you are working with imperative expectations, then blocking might be the best option. I'd have to get it loaded up in an IDE and play with it to confirm.

@scottfrederick scottfrederick added this to the 1.0.0.M1 milestone Nov 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants