[GitHub] [beam] KhaninArtur commented on a change in pull request #16277: [BEAM-13124][Playground] Create readiness endpoint

[GitBox <gi...@apache.org>]

KhaninArtur commented on a change in pull request #16277:
URL: https://github.com/apache/beam/pull/16277#discussion_r772404494



##########
File path: playground/backend/cmd/server/http.go
##########
@@ -33,3 +49,28 @@ func listenHttp(ctx context.Context, errChan chan error, envs environment.Networ
 		return
 	}
 }
+
+// isReady checks the number of already working code processing.
+//  It counts by the number of the /path/to/workingDir/executable_files/{pipelineId} folders.
+// If it is equals or more than numOfParallelJobs, then returns false.
+// If it is less than numOfParallelJobs, then returns true.
+func isReady(workingDir string, numOfParallelJobs int) bool {
+	// TODO add getting of dir executable_files from environments.
+	baseFileFolder := filepath.Join(workingDir, "executable_files")
+	_, err := os.Stat(baseFileFolder)
+	if os.IsNotExist(err) {
+		return true
+	}
+
+	dirEntries, err := os.ReadDir(baseFileFolder)
+	if err != nil {
+		logger.Errorf("Readiness: Error during read %s: %s", baseFileFolder, err.Error())
+		return false
+	}
+
+	if len(dirEntries) >= numOfParallelJobs {
+		logger.Errorf("Readiness: Count of code processing is equals or more than possible: %d / %d", len(dirEntries), numOfParallelJobs)

Review comment:
       ```suggestion
   		logger.Errorf("Readiness: Count of code processing is equal or more than possible: %d / %d", len(dirEntries), numOfParallelJobs)
   ```

##########
File path: playground/backend/cmd/server/http.go
##########
@@ -33,3 +49,28 @@ func listenHttp(ctx context.Context, errChan chan error, envs environment.Networ
 		return
 	}
 }
+
+// isReady checks the number of already working code processing.
+//  It counts by the number of the /path/to/workingDir/executable_files/{pipelineId} folders.
+// If it is equals or more than numOfParallelJobs, then returns false.
+// If it is less than numOfParallelJobs, then returns true.
+func isReady(workingDir string, numOfParallelJobs int) bool {
+	// TODO add getting of dir executable_files from environments.
+	baseFileFolder := filepath.Join(workingDir, "executable_files")
+	_, err := os.Stat(baseFileFolder)
+	if os.IsNotExist(err) {
+		return true
+	}
+
+	dirEntries, err := os.ReadDir(baseFileFolder)
+	if err != nil {
+		logger.Errorf("Readiness: Error during read %s: %s", baseFileFolder, err.Error())
+		return false
+	}
+
+	if len(dirEntries) >= numOfParallelJobs {

Review comment:
       What happens if `numOfParallelJobs` = 0? I think we should avoid such situations

##########
File path: playground/backend/cmd/server/http.go
##########
@@ -33,3 +49,28 @@ func listenHttp(ctx context.Context, errChan chan error, envs environment.Networ
 		return
 	}
 }
+
+// isReady checks the number of already working code processing.
+//  It counts by the number of the /path/to/workingDir/executable_files/{pipelineId} folders.
+// If it is equals or more than numOfParallelJobs, then returns false.
+// If it is less than numOfParallelJobs, then returns true.
+func isReady(workingDir string, numOfParallelJobs int) bool {
+	// TODO add getting of dir executable_files from environments.
+	baseFileFolder := filepath.Join(workingDir, "executable_files")

Review comment:
       "executable_files" seems like a constant

##########
File path: playground/backend/internal/environment/environment_service.go
##########
@@ -137,6 +140,17 @@ func GetNetworkEnvsFromOsEnvs() (*NetworkEnvs, error) {
 func ConfigureBeamEnvs(workDir string) (*BeamEnvs, error) {
 	sdk := pb.Sdk_SDK_UNSPECIFIED
 	preparedModDir, modDirExist := os.LookupEnv(preparedModDirKey)
+
+	numOfParallelJobs := defaultNumOfParallelJobs
+	if value, present := os.LookupEnv(numOfParallelJobsKey); present {
+		convertedValue, err := strconv.Atoi(value)
+		if err != nil {
+			logger.Errorf("Incorrect value for %s. Should be integer. Will be used default value: %d", numOfParallelJobsKey, defaultNumOfParallelJobs)
+		} else {
+			numOfParallelJobs = convertedValue

Review comment:
       In addition to the comment above, we should check, that the value is strictly positive

##########
File path: playground/backend/internal/environment/environment_service.go
##########
@@ -137,6 +140,17 @@ func GetNetworkEnvsFromOsEnvs() (*NetworkEnvs, error) {
 func ConfigureBeamEnvs(workDir string) (*BeamEnvs, error) {
 	sdk := pb.Sdk_SDK_UNSPECIFIED
 	preparedModDir, modDirExist := os.LookupEnv(preparedModDirKey)
+
+	numOfParallelJobs := defaultNumOfParallelJobs
+	if value, present := os.LookupEnv(numOfParallelJobsKey); present {
+		convertedValue, err := strconv.Atoi(value)
+		if err != nil {
+			logger.Errorf("Incorrect value for %s. Should be integer. Will be used default value: %d", numOfParallelJobsKey, defaultNumOfParallelJobs)

Review comment:
       @ilya-kozyrev why? I think default value is a good strategy




-- 
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: github-unsubscribe@beam.apache.org

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