[GitHub] [shardingsphere-on-cloud] mlycore commented on a diff in pull request #312: add event and status.phase to control resource create check

["mlycore (via GitHub)" <gi...@apache.org>]

mlycore commented on code in PR #312:
URL: https://github.com/apache/shardingsphere-on-cloud/pull/312#discussion_r1168418133


##########
shardingsphere-operator/api/v1alpha1/shardingsphere_chaos_types.go:
##########
@@ -77,24 +75,21 @@ const (
 	UnKnown      ChaosCondition = "UnKnown"

Review Comment:
   Rename to `Unknown` since it is a common word



##########
shardingsphere-operator/api/v1alpha1/shardingsphere_chaos_types.go:
##########
@@ -77,24 +75,21 @@ const (
 	UnKnown      ChaosCondition = "UnKnown"
 )
 
-// Jobschedule Show current job progress
-type Jobschedule string
-
-const (
-	JobCreating Jobschedule = "JobCreating"
-	JobFailed   Jobschedule = "JobFailed"
-	JobFinish   Jobschedule = "JobFinish"
-)
-
 // ShardingSphereChaosStatus defines the actual state of ShardingSphereChaos
 type ShardingSphereChaosStatus struct {
 	ChaosCondition ChaosCondition `json:"chaosCondition"`
-	//todo
-	//InjectStatus   Jobschedule         `json:"InjectStatus"`
-	//todo
-	//VerifyStatus   Jobschedule         `json:"VerifyStatus"`
+	Phase          Phase          `json:"phase"`
 }
 
+type Phase string
+
+var (
+	PhaseBeforeExperiment Phase = "before experiment"

Review Comment:
   The phase should be a single word to describe, so it will be easier to know the phase transition procedure.



##########
shardingsphere-operator/build/tools/Dockerfile:
##########
@@ -24,8 +24,8 @@ ENV ZOOKEEPER_DOWNLOAD_URL https://dlcdn.apache.org/zookeeper/zookeeper-3.7.1/ap
 ENV ZOOKEEPER_DIR /app/zookeeper
 WORKDIR /app
 RUN mkdir -p "/app/start" && chmod -R 777 /app/start
-CMD ["tail -f /dev/null"]
 ENTRYPOINT ["sh","-c"]

Review Comment:
   The `entrypoint` should be placed at the last line of the Dockerfile. It is the `entry point` of this docker image. Any commands wants to be executed in this image could be injected by the runtime platform.



##########
shardingsphere-operator/pkg/controllers/shardingsphere_chaos_controller.go:
##########
@@ -40,7 +43,9 @@ import (
 
 const (
 	ShardingSphereChaosControllerName = "shardingsphere-chaos-controller"
-	ssChaosDefaultEnqueueTime         = 5 * time.Second
+	ssChaosDefaultEnqueueTime         = 10 * time.Second
+	defaultCreatedMessage             = " is created successfully"

Review Comment:
   I may not suggest to put these messages as consts. Actually, this could be wrapped in a function matter.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org