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

Refactor cloudlaunch for use as a library #137

Closed
2 tasks done
machristie opened this issue Feb 7, 2018 · 4 comments
Closed
2 tasks done

Refactor cloudlaunch for use as a library #137

machristie opened this issue Feb 7, 2018 · 4 comments

Comments

@machristie
Copy link
Collaborator

machristie commented Feb 7, 2018

Create a layer (actions?) in cloudlaunch that can be called directly as a library. This layer would sit below serializers/viewsets.

First step is to create some unit tests over the existing code to guard against regressions.

TODO

  • DeploymentTask tests
  • Deployment tests
  • move only one launch task logic to model
  • factor out DeploymentSerializer.create logic to an action
  • what's the desired API when using cloudlaunch as a library?
@machristie
Copy link
Collaborator Author

We decided against trying to use cloudlaunch as a library. To do so we would need to create a new API layer in cloudlaunch that sits above the Django models and Celery tasks but below the DRF viewsets and serializers. This API layer would define the single API of CloudLaunch and the DRF viewsets and serializers would then just map that API to a RESTful interface. Such a single API would prevent divergence between the REST API and using cloudlaunch as a library. However, this seems a good bit of work just to get to the point of being able to call cloudlaunch directly. Also, the DRF serializers assume they are working with Django models directly so it's unclear how having a layer between the serializers and the models would work or if we would need to explicit map the serializers, increasing the amount of work to be done.

Our current thought is to call cloudlaunch internally by its REST API. For this we'll use the API wrapper in cloudlaunch-cli. This has the beneficial side effect of ending up with an improved API wrapper that the cloudlaunch-cli can also use.

I'm going to go ahead and finish the tests that I created in the linked pull request and I hope to get that merged.

@afgane
Copy link
Contributor

afgane commented Feb 26, 2018

Such a single API would prevent divergence between the REST API and using cloudlaunch as a library.

Didn't we think the opposite was a possibility? For example, some functionality could be implemented at the lowest, library level but not exposed via the serializers to the REST API, which would cause divergence in what the library exposes vs. what's available via REST.

Also, so we can recall this later - one more bit we discussed was auth. It's likely that auth would be simpler if everything was contained within CloudLaunch vs. going through REST and the CLI, especially when different classes of users need to be supported (e.g., superuser vs. standard user). Hopefully we can outsource this to something like Keycloak in the future anyhow.

@machristie
Copy link
Collaborator Author

Didn't we think the opposite was a possibility? For example, some functionality could be implemented at the lowest, library level but not exposed via the serializers to the REST API, which would cause divergence in what the library exposes vs. what's available via REST.

Yes that's right. The set of functionality exposed via REST would be a subset of what is available in lower library level. In that sentence I meant that there wouldn't be divergence of how the functionality is implemented, for example, the API layer would provide a way to create a RESTART task, instead of it being implemented one way in the REST code and another way when used as a library.

@machristie
Copy link
Collaborator Author

This issue can be closed now. This work has essentially moved to cloudlaunch-cli, see CloudVE/cloudlaunch-cli#4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants