Skip to content

Commit

Permalink
adjust comments
Browse files Browse the repository at this point in the history
  • Loading branch information
helinwang committed Oct 11, 2017
1 parent 3199e78 commit 0b4deba
Showing 1 changed file with 17 additions and 19 deletions.
36 changes: 17 additions & 19 deletions go/controller/k8s/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,11 @@ import (
"context"

log "github.com/sirupsen/logrus"
// kubernetes api utilities

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/fields"
v1beta1 "k8s.io/client-go/pkg/apis/extensions/v1beta1"
"k8s.io/kubernetes/pkg/api"
// kubernetes go client

This comment has been minimized.

Copy link
@helinwang

helinwang Oct 11, 2017

Author Collaborator

我觉得开发者可以通过import路径看出来是干什么的,这个comment貌似不是很必要。


"k8s.io/client-go/kubernetes"
"k8s.io/client-go/rest"
"k8s.io/client-go/tools/cache"
Expand All @@ -44,7 +42,6 @@ import (

// Controller for dispatching TrainingJob resource.
type Controller struct {
// logger *logrus.Logger
client *rest.RESTClient
clientset *kubernetes.Clientset
autoscaler *paddlejob.Autoscaler
Expand All @@ -70,13 +67,11 @@ func NewController(config *rest.Config) (*Controller, error) {

// Run start to watch kubernetes events and do handlers.
func (c *Controller) Run(ctx context.Context) error {
// start controller watch

This comment has been minimized.

Copy link
@helinwang

helinwang Oct 11, 2017

Author Collaborator

下面一行的代码好像已经说了这个comment要讲的,我觉得如果代码说清楚了,就不用comment了,因为代码可以编译,但是comment可能会过时。

This comment has been minimized.

Copy link
@typhoonzero

typhoonzero Oct 12, 2017

Collaborator

有道理!看来代码足够清晰是首要的!

err := c.startWatch(ctx)
if err != nil {
return err
}
// start autoscaler
// call c.autoscaler.Monitor(nil) to start
// TODO(helin): start autoscaler

<-ctx.Done()
return ctx.Err()
Expand All @@ -93,9 +88,9 @@ func (c *Controller) startWatch(ctx context.Context) error {
source,
&paddlejob.TrainingJob{},

// resyncPeriod
// Every resyncPeriod, all resources in the cache will retrigger events.

This comment has been minimized.

Copy link
@helinwang

helinwang Oct 11, 2017

Author Collaborator

Adjusted the comment width.

// Set to 0 to disable the resync.
// resyncPeriod: Every resyncPeriod, all resources in
// the cache will retrigger events. Set to 0 to
// disable the resync.
0,

// TrainingJob custom resource event handlers.
Expand All @@ -111,28 +106,31 @@ func (c *Controller) startWatch(ctx context.Context) error {

func (c *Controller) onAdd(obj interface{}) {
job := obj.(*paddlejob.TrainingJob)
log.Debugln("onAdd.")
log.Debugln(job)
// call c.client.Put() to send REST call to api-server
log.Debugln("onAdd: %v", *job)
// TODO: call c.client.Put() to send REST call to api-server
namespace := job.ObjectMeta.Namespace
jobname := job.ObjectMeta.Name
rslist, err := c.clientset.ExtensionsV1beta1().ReplicaSets(namespace).List(metav1.ListOptions{})
if err != nil {
log.Errorln(err)
}
log.Debugln(rslist)
log.Debugln("Existing replicasets: %v", *rslist)
exist := false
for _, item := range rslist.Items {
if item.Name == jobname {
exist = true
break
}
}

if exist {
log.Errorln("Job name already exists:", jobname)
return
}

// generate a pserver replicaset resource according to "TrainingJob" resource specs.
pserverRS := v1beta1.ReplicaSet{}
if !exist {
c.clientset.ExtensionsV1beta1().ReplicaSets(namespace).Create(&pserverRS)
}
c.clientset.ExtensionsV1beta1().ReplicaSets(namespace).Create(&pserverRS)
}

func (c *Controller) onUpdate(oldObj, newObj interface{}) {
Expand All @@ -141,12 +139,12 @@ func (c *Controller) onUpdate(oldObj, newObj interface{}) {
log.Debugln(oldjob)
log.Debugln(newjob)
log.Debugln("onUpdate.")
// call c.client.Put() to update resource
// TODO: call c.client.Put() to update resource
}

func (c *Controller) onDelete(obj interface{}) {
job := obj.(*paddlejob.TrainingJob)
log.Debugln("onDelete.")
log.Debugln(job)
// call c.client.Delete()
// TODO: call c.client.Delete()
}

1 comment on commit 0b4deba

@typhoonzero
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for these code cleanups!

Please sign in to comment.