Skip to content

Fixed Queue and Refactored Driver#88

Merged
jeda merged 7 commits intocrybapp:masterfrom
SolCryb:NewQueue
Feb 16, 2020
Merged

Fixed Queue and Refactored Driver#88
jeda merged 7 commits intocrybapp:masterfrom
SolCryb:NewQueue

Conversation

@Soliel
Copy link
Copy Markdown
Contributor

@Soliel Soliel commented Feb 16, 2020

  • Replaced old queue with a new one
  • New queue is decoupled from all drivers
  • Added an IPortalDriver interface
  • Changed portal.driver.ts to PortalManager.service.ts to more accurately reflect it's purpose
  • Deleted Router.ts
  • Updated drivers to implement from IPortalDriver
  • Standardized the driver interface
  • Added isSpaceAvailable to all drivers
  • Queue will verify with the driver that space is available on the chosen platform
  • Drivers are now registered in ServiceManager
  • Driver is selected once during first call to PortalManager
  • Implemented unit testing for the new queue

@Soliel Soliel requested a review from jeda February 16, 2020 16:55
Copy link
Copy Markdown
Member

@jeda jeda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've done some code style changes and such. This seems fine anyway.

One thing I have on mind though, that some providers like DigitalOcean allow to access the maximum droplet limit via its API, which could be used to give an answer for availability instead of simply falling always for true.

Plus that, a limit that could be set in an environment variable would be nice.


One thing I didn't comment you, is that I think though, as drivers are re-factored, I feel that we could just export the drivers by default, so we can simply count on the default import instead of doing specific ones. There should not be a need to do multiple exports anyway, as drivers should be kept separate from each one.

@Soliel
Copy link
Copy Markdown
Contributor Author

Soliel commented Feb 16, 2020

I've done some code style changes and such. This seems fine anyway.

One thing I have on mind though, that some providers like DigitalOcean allow to access the maximum droplet limit via its API, which could be used to give an answer for availability instead of simply falling always for "true".

Plus that, a limit that could be set in an environment variable would be nice.

One thing I didn't comment you, is that I think though, as drivers are re-factored, I feel that we could just export the drivers by default, so we can simply count on the default import instead of doing specific ones. There should not be a need to do multiple exports anyway, as drivers should be kept separate from each one.

I agree with the point about exporting as default. We can go ahead and do that. For existing drivers I didn't want to actually finish implementing the isSpaceAvailable because it would have taken a bunch of research specific to each driver. We should open some issues to improve them but other than that it's fine to.

Copy link
Copy Markdown
Member

@jeda jeda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's all good to go then.

I'll open a new issue in order to actually look for availability plus more configuration for that. The base is already there, anyway.

@jeda jeda merged commit 4d6f32a into crybapp:master Feb 16, 2020
@Soliel Soliel deleted the NewQueue branch February 17, 2020 13:11
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

Successfully merging this pull request may close these issues.

2 participants