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

LBConfigProvider is inflexible #29

Closed
laixintao opened this issue Dec 5, 2021 · 7 comments · Fixed by #41
Closed

LBConfigProvider is inflexible #29

laixintao opened this issue Dec 5, 2021 · 7 comments · Fixed by #41

Comments

@laixintao
Copy link
Collaborator

My original idea for the provider is that:

A provider can be defined in a single file, a provider's exceptions, implementations, and config definitions, all can be defined in a single file.

So that it would be easy to maintain. And also, a provider can be implemented outside lobbyboy's codebase. (let's say, one can upload his provider to pypi, and other users can install them via pip install lobbyboy-aws-ec2-provider, then lobbyboy can load it.

And one can add custom configs to his provider.

The current problem is, we have to update the LBConfigProvider if one wants to add more fields to his provider config. It is not possible to do so without updating lobbyboy's source code.

@laixintao
Copy link
Collaborator Author

@luxiaba

I think we can do this by searching subclasses of LBConfigProvider, if there is, then load origin config into that subclass instead of LBConfigProvider

@luxiaba
Copy link
Collaborator

luxiaba commented Dec 5, 2021

  1. Let developers write code flexibly and freely as much as possible.
  2. Make lobbyboy has a simple structure, and control all providers to ensure the consistency of services or operations for users.
    i think these two things are a little contradictory, we need find a balance🤣

@luxiaba
Copy link
Collaborator

luxiaba commented Dec 5, 2021

if we let developers code freely, users can install it from pypi like pip install lobbyboy-provider-XXX, i am worried about the following:

  • code security
    we don't know when and what the this provider actually did, for example, the user provided his digitalocean token, and then would the this provider delete the user's server, or stole data from server?

  • management (configuration, reading and management of server directory)

  • consistency of behavior
    i think this problem maybe that the abstract level of the provider is not enough. the current process in the provider is as follows:

    CreateServer -> SaveToLocal -> WaitingServerUp -> ReturnToLB(LBServerMeta) -> ... -> DestroyServer

    actually, we want the provider only provides basic method support (create_server, destroy_server), and all other things should be done by lobbyboy.

@luxiaba
Copy link
Collaborator

luxiaba commented Dec 5, 2021

so i prefer this method: use pip install lobbyboy[provider-digitalocean] to load provider, which makes providers optional and controllable.
in addition, streamline the structure of the provider, reduce dependence on lobbyboy, and let lobbyboy manage the whole process:

  • for example, adding an extra method to check whether the server is ready or not instead of provider to do and send many msgs to user.
  • restrict the provider sending msg to users by the channel(too restrictive manybe).

some thoughts tonight, maybe not help to this issue, write down first😂

@laixintao
Copy link
Collaborator Author

good point. 👍

code security

But I don't think that will be a problem, when you install any packages from pypi, you already trust them. Actually, even when you install a vim plugin from github, it already can do all the bad things to your computer.

When the user install lobbyboy-xx, he/she already knows that this package was not provided by lobbyboy.

So there is nothing different, if you use open-source software, you have to inspect its reputation, quality, documentations, etc.

management (configuration, reading and management of server directory)

Provider can always define his own available configurations.

consistency of behavior

Yes, we can limit it by only calling provider's two API.

@messense
Copy link
Collaborator

messense commented Dec 8, 2021

Hitting this when trying to address #37 (comment)

@messense
Copy link
Collaborator

messense commented Dec 8, 2021

IMO maybe a Provider should have a config field which is a dataclass, we can deserialize it from config.toml

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 a pull request may close this issue.

3 participants