-
Notifications
You must be signed in to change notification settings - Fork 76
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
Split backend and paddlejob abstraction #374
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/kubernetes/minikube
我们需要一个单测系统,特别是多个后端的情况下。
import utils | ||
import volume | ||
|
||
# FIXME(typhoonzero): need a base class to define the interfaces? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
如果是想抽象多个后端的接口,Interface层还是需要的。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
所以目前是FIXME,后续增加即可。目前没必要嵌套那么多层。
from kubernetes import client, config | ||
from kubernetes.client.rest import ApiException | ||
# FIXME(typhoonzero): still need to import settings | ||
from django.conf import settings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
不利于单测
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
所以加了FIXME,目前只能把配置都放一个settings里,分成多个配置文件,会让部署很麻烦。
另外即使要加单测,from django.conf import settings
也是有方法可以使用的。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fix #373
After this PR we can put more backends to it to support more clusters like kubernetes.