[
https://issues.apache.org/jira/browse/BEAM-13308?focusedWorklogId=702256&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-702256
]
ASF GitHub Bot logged work on BEAM-13308:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 30/Dec/21 09:35
Start Date: 30/Dec/21 09:35
Worklog Time Spent: 10m
Work Description: AydarZaynutdinov commented on a change in pull request
#16384:
URL: https://github.com/apache/beam/pull/16384#discussion_r776622798
##########
File path: playground/backend/internal/fs_tool/fs.go
##########
@@ -57,14 +57,14 @@ type LifeCycle struct {
// NewLifeCycle returns a corresponding LifeCycle depending on the given SDK.
// workingDir should be existed and be prepared to create/delete/modify
folders into him.
Review comment:
We can remove this comment
##########
File path: playground/backend/internal/fs_tool/fs_test.go
##########
@@ -144,7 +145,7 @@ func TestLifeCycle_CreateExecutableFile(t *testing.T) {
func TestLifeCycle_CreateFolders(t *testing.T) {
pipelineId := uuid.New()
- baseFileFolder := fmt.Sprintf("%s_%s", baseFileFolder, pipelineId)
+ baseFileFolder := fmt.Sprintf("%s_%s", pipelinesFolder, pipelineId)
Review comment:
```suggestion
baseFileFolder := fmt.Sprintf("%s/%s", pipelinesFolder, pipelineId)
```
##########
File path: playground/backend/internal/fs_tool/fs_test.go
##########
@@ -300,7 +302,7 @@ func TestNewLifeCycle(t *testing.T) {
func TestLifeCycle_GetAbsoluteExecutableFilePath(t *testing.T) {
pipelineId := uuid.New()
- baseFileFolder := fmt.Sprintf("%s_%s", baseFileFolder, pipelineId)
+ baseFileFolder := fmt.Sprintf("%s_%s", pipelinesFolder, pipelineId)
Review comment:
```suggestion
baseFileFolder := fmt.Sprintf("%s/%s", pipelinesFolder, pipelineId)
```
##########
File path: playground/backend/internal/fs_tool/fs_test.go
##########
@@ -186,7 +187,7 @@ func TestLifeCycle_CreateFolders(t *testing.T) {
func TestLifeCycle_DeleteFolders(t *testing.T) {
pipelineId := uuid.New()
- baseFileFolder := fmt.Sprintf("%s_%s", baseFileFolder, pipelineId)
+ baseFileFolder := fmt.Sprintf("%s_%s", pipelinesFolder, pipelineId)
Review comment:
```suggestion
baseFileFolder := fmt.Sprintf("%s/%s", pipelinesFolder, pipelineId)
```
##########
File path: playground/backend/internal/fs_tool/fs_test.go
##########
@@ -348,7 +350,7 @@ func TestLifeCycle_GetAbsoluteExecutableFilePath(t
*testing.T) {
func TestLifeCycle_GetAbsoluteExecutableFilesFolderPath(t *testing.T) {
pipelineId := uuid.New()
- baseFileFolder := fmt.Sprintf("%s_%s", baseFileFolder, pipelineId)
+ baseFileFolder := fmt.Sprintf("%s_%s", pipelinesFolder, pipelineId)
Review comment:
```suggestion
baseFileFolder := fmt.Sprintf("%s/%s", pipelinesFolder, pipelineId)
```
##########
File path: playground/backend/internal/fs_tool/go_fs_test.go
##########
@@ -18,20 +18,23 @@ package fs_tool
import (
"fmt"
"github.com/google/uuid"
+ "path/filepath"
"reflect"
"testing"
)
func Test_newGoLifeCycle(t *testing.T) {
pipelineId := uuid.New()
workingDir := "workingDir"
- baseFileFolder := fmt.Sprintf("%s/%s/%s", workingDir, baseFileFolder,
pipelineId)
+ preparedPipelinesFolder := filepath.Join(workingDir, pipelinesFolder)
+ baseFileFolder := fmt.Sprintf("%s/%s", preparedPipelinesFolder,
pipelineId)
Review comment:
```suggestion
baseFileFolder := filepath.Join(workingDir, pipelinesFolder, pipelineId)
```
##########
File path: playground/backend/internal/fs_tool/java_fs_test.go
##########
@@ -27,13 +27,15 @@ import (
func Test_newJavaLifeCycle(t *testing.T) {
pipelineId := uuid.New()
workingDir := "workingDir"
- baseFileFolder := fmt.Sprintf("%s/%s/%s", workingDir, baseFileFolder,
pipelineId)
+ preparedPipelinesFolder := filepath.Join(workingDir, pipelinesFolder)
+ baseFileFolder := fmt.Sprintf("%s/%s", preparedPipelinesFolder,
pipelineId)
Review comment:
```suggestion
baseFileFolder := filepath.Join(workingDir, pipelinesFolder, pipelineId)
```
##########
File path: playground/backend/internal/fs_tool/java_fs_test.go
##########
@@ -45,8 +47,9 @@ func Test_newJavaLifeCycle(t *testing.T) {
// As a result, want to receive an expected java life
cycle.
name: "newJavaLifeCycle",
args: args{
- pipelineId: pipelineId,
- workingDir: workingDir,
+ pipelineId: pipelineId,
+ workingDir: workingDir,
+ pipelinesFolder: preparedPipelinesFolder,
Review comment:
```suggestion
pipelinesFolder: baseFileFolder,
```
##########
File path: playground/backend/internal/preparators/java_preparators_test.go
##########
@@ -37,7 +39,7 @@ func Test_replace(t *testing.T) {
if err != nil {
panic(err)
}
- lc, _ := fs_tool.NewLifeCycle(pb.Sdk_SDK_JAVA, uuid.New(),
filepath.Join(path, "temp"))
+ lc, _ := fs_tool.NewLifeCycle(pb.Sdk_SDK_JAVA, uuid.New(),
filepath.Join(path, "temp", pipelinesFolder))
Review comment:
It is not necessary. `fs_tool.NewLifeCycle` receive any directory as the
last argument and it doesn't matter if it will be `temp/executable_files` or
just `executable_files`.
##########
File path: playground/backend/internal/fs_tool/fs_test.go
##########
@@ -392,7 +394,7 @@ func TestLifeCycle_GetAbsoluteExecutableFilesFolderPath(t
*testing.T) {
func TestLifeCycle_ExecutableName(t *testing.T) {
pipelineId := uuid.New()
workingDir := "workingDir"
Review comment:
It seems that we do not need this variable
##########
File path: playground/backend/internal/fs_tool/fs_test.go
##########
@@ -71,7 +72,7 @@ func teardown() {
func TestLifeCycle_CreateExecutableFile(t *testing.T) {
pipelineId := uuid.New()
- baseFileFolder := fmt.Sprintf("%s_%s", baseFileFolder, pipelineId)
+ baseFileFolder := fmt.Sprintf("%s_%s", pipelinesFolder, pipelineId)
Review comment:
```suggestion
baseFileFolder := fmt.Sprintf("%s/%s", pipelinesFolder, pipelineId)
```
##########
File path: playground/backend/internal/fs_tool/go_fs_test.go
##########
@@ -43,8 +46,8 @@ func Test_newGoLifeCycle(t *testing.T) {
// As a result, want to receive an expected go life
cycle.
name: "newGoLifeCycle",
args: args{
- pipelineId: pipelineId,
- workingDir: workingDir,
+ pipelineId: pipelineId,
+ pipelinesFolder: preparedPipelinesFolder,
Review comment:
```suggestion
pipelinesFolder: baseFileFolder,
```
##########
File path: playground/backend/internal/fs_tool/java_fs_test.go
##########
@@ -45,8 +47,9 @@ func Test_newJavaLifeCycle(t *testing.T) {
// As a result, want to receive an expected java life
cycle.
name: "newJavaLifeCycle",
args: args{
- pipelineId: pipelineId,
- workingDir: workingDir,
+ pipelineId: pipelineId,
+ workingDir: workingDir,
Review comment:
This field isn't used in the test case. So we can remove it.
##########
File path: playground/backend/internal/fs_tool/java_fs_test.go
##########
@@ -27,13 +27,15 @@ import (
func Test_newJavaLifeCycle(t *testing.T) {
pipelineId := uuid.New()
workingDir := "workingDir"
- baseFileFolder := fmt.Sprintf("%s/%s/%s", workingDir, baseFileFolder,
pipelineId)
+ preparedPipelinesFolder := filepath.Join(workingDir, pipelinesFolder)
+ baseFileFolder := fmt.Sprintf("%s/%s", preparedPipelinesFolder,
pipelineId)
srcFileFolder := baseFileFolder + "/src"
binFileFolder := baseFileFolder + "/bin"
type args struct {
- pipelineId uuid.UUID
- workingDir string
+ pipelineId uuid.UUID
+ workingDir string
Review comment:
We do not need this field here.
##########
File path: playground/backend/internal/fs_tool/go_fs_test.go
##########
@@ -18,20 +18,23 @@ package fs_tool
import (
"fmt"
"github.com/google/uuid"
+ "path/filepath"
"reflect"
"testing"
)
func Test_newGoLifeCycle(t *testing.T) {
pipelineId := uuid.New()
workingDir := "workingDir"
- baseFileFolder := fmt.Sprintf("%s/%s/%s", workingDir, baseFileFolder,
pipelineId)
+ preparedPipelinesFolder := filepath.Join(workingDir, pipelinesFolder)
+ baseFileFolder := fmt.Sprintf("%s/%s", preparedPipelinesFolder,
pipelineId)
srcFileFolder := baseFileFolder + "/src"
binFileFolder := baseFileFolder + "/bin"
type args struct {
- pipelineId uuid.UUID
- workingDir string
+ pipelineId uuid.UUID
+ workingDir string
Review comment:
We do not need this field here.
##########
File path: playground/backend/internal/fs_tool/python_fs_test.go
##########
@@ -41,8 +43,8 @@ func Test_newPythonLifeCycle(t *testing.T) {
// As a result, want to receive an expected python life
cycle.
name: "newPythonLifeCycle",
args: args{
- pipelineId: pipelineId,
- workingDir: workingDir,
+ pipelineId: pipelineId,
+ pipelinesFolder: preparedPipelinesFolder,
Review comment:
```suggestion
pipelinesFolder: baseFileFolder,
```
##########
File path: playground/backend/internal/fs_tool/python_fs_test.go
##########
@@ -18,18 +18,20 @@ package fs_tool
import (
"fmt"
"github.com/google/uuid"
+ "path/filepath"
"reflect"
"testing"
)
func Test_newPythonLifeCycle(t *testing.T) {
pipelineId := uuid.New()
workingDir := "workingDir"
- baseFileFolder := fmt.Sprintf("%s/%s/%s", workingDir, baseFileFolder,
pipelineId)
+ preparedPipelinesFolder := filepath.Join(workingDir, pipelinesFolder)
+ baseFileFolder := fmt.Sprintf("%s/%s", preparedPipelinesFolder,
pipelineId)
Review comment:
```suggestion
baseFileFolder := filepath.Join(workingDir, pipelinesFolder, pipelineId)
```
##########
File path: playground/backend/internal/preparators/java_preparators_test.go
##########
@@ -118,7 +120,7 @@ func Test_changeJavaTestFileName(t *testing.T) {
if err != nil {
panic(err)
}
- lc, _ := fs_tool.NewLifeCycle(pb.Sdk_SDK_JAVA, uuid.New(),
filepath.Join(path, "temp"))
+ lc, _ := fs_tool.NewLifeCycle(pb.Sdk_SDK_JAVA, uuid.New(),
filepath.Join(path, "temp", pipelinesFolder))
Review comment:
ditto
##########
File path:
playground/backend/internal/setup_tools/life_cycle/life_cycle_setuper_test.go
##########
@@ -47,16 +48,17 @@ func TestSetup(t *testing.T) {
}
defer os.RemoveAll(workingDir)
- lc, err := fs_tool.NewLifeCycle(playground.Sdk_SDK_JAVA,
successPipelineId, workingDir)
+ lc, err := fs_tool.NewLifeCycle(playground.Sdk_SDK_JAVA,
successPipelineId, filepath.Join(workingDir, pipelinesFolder))
Review comment:
It is not necessary. `fs_tool.NewLifeCycle` receives any directory as
the last argument so it doesn't matter if it will `workingDir/executable_files`
or just `workingDir`.
##########
File path:
playground/backend/internal/setup_tools/life_cycle/life_cycle_setuper.go
##########
@@ -34,14 +35,13 @@ const (
javaLogFilePlaceholder = "{logFilePath}"
goModFileName = "go.mod"
goSumFileName = "go.sum"
- baseFileFolder = "executable_files"
)
// Setup returns fs_tool.LifeCycle.
// Also, prepares files and folders needed to code processing according to sdk
-func Setup(sdk pb.Sdk, code string, pipelineId uuid.UUID, workingDir string,
preparedModDir string) (*fs_tool.LifeCycle, error) {
+func Setup(sdk pb.Sdk, code string, pipelineId uuid.UUID, workingDir,
pipelinesFolder, preparedModDir string) (*fs_tool.LifeCycle, error) {
Review comment:
Oh I see that we need to use `workingDir` for the `prepareJavaFiles()`.
So yes let's leave it as you did.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 702256)
Time Spent: 50m (was: 40m)
> [Playground] Getting baseFileFolder from environment
> ----------------------------------------------------
>
> Key: BEAM-13308
> URL: https://issues.apache.org/jira/browse/BEAM-13308
> Project: Beam
> Issue Type: Task
> Components: beam-playground
> Reporter: Pavel Avilov
> Assignee: Pavel Avilov
> Priority: P3
> Labels: beam-playground-backend
> Time Spent: 50m
> Remaining Estimate: 0h
>
> Need to get the baseFileFolder value from the environment variable (now
> baseFileFolder is a constant). Also in the dockerfile for go SDK change the
> command "go mod init executable_files" to "go mod init
> ${PIPELINES_FOLDER_NAME}", where "PIPELINES_FOLDER_NAME" is the name of env.
> variable for baseFileFolder.
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
