Skip to content
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

feat: multi task import #117

Merged
merged 2 commits into from
Mar 16, 2022
Merged

feat: multi task import #117

merged 2 commits into from
Mar 16, 2022

Conversation

xjlgod
Copy link
Contributor

@xjlgod xjlgod commented Feb 21, 2022

No description provided.

@CLAassistant
Copy link

CLAassistant commented Feb 21, 2022

CLA assistant check
All committers have signed the CLA.

server/README.md Outdated
Comment on lines 32 to 35
| HandleImportAction | /api-nebula/task/import/action | POST |
| QueryImportStats | /api-nebula/task/import/stats | POST |
| DownloadConfig | /api-nebula/task/import/config | POST |
| DownloadLog | /api-nebula/task/import/log | GET |
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use restful style api?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I will modify it

server/README.md Outdated
address: host
postStart: null
preStop: null
logPath: E:\NebulaProject\新建文件夹 (4)\nebula-studio\server\data\tasks\1/import.log
Copy link
Contributor

Choose a reason for hiding this comment

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

Do not use Chinese.

server/README.md Outdated

The request :

http://localhost:9000/api-nebula/task/import/log/?pathName=E:\NebulaProject\nebula-studio\server\data\tasks\1\err\err 1.log
Copy link
Contributor

Choose a reason for hiding this comment

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

Here is what to download logs? Why not use the task id nor path?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because there is need to download two types of files with same id.

Copy link
Contributor

Choose a reason for hiding this comment

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

The client should not know the path of the service.
You can use parameters and paths to differentiate.

server/README.md Outdated
Comment on lines 345 to 346
"bbbbbbbbbbb",
"ccccccccccc"
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this?

@@ -266,9 +394,51 @@ Response:
"code": 0,
"message": "",
"data": {
"taskDir": "E:\\NebulaProject\\test\\nebula-studio\\server\\data\\tasks",
"uploadDir": "E:\\NebulaProject\\test\\nebula-studio\\server\\data\\upload"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why expose this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To get all files from this dir

Copy link
Contributor

Choose a reason for hiding this comment

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

Comment on lines 171 to 182
if err != nil {
return err
}
if err != nil {
return err
}
Copy link
Contributor

Choose a reason for hiding this comment

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

dup?

Comment on lines 177 to 186
err = ioutil.WriteFile(path, []byte(utf8String), 0666)
if err != nil {
return err
}
return nil
Copy link
Contributor

Choose a reason for hiding this comment

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

You can just return ioutil.WriteFile(path, []byte(utf8String), 0666).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok

Comment on lines 276 to 279
ctx.SendFile(configPath, "config.yaml")
return base.Response{
Code: base.Success,
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any abnormality call ctx.JSON(&result) after ctx.SendFile ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Forget?

if index == -1 {
index = strings.LastIndex(path, "\\")
}
filename := path[index:]
Copy link
Contributor

Choose a reason for hiding this comment

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

How about index == -1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The path may be under linux or windows env.

Copy link
Contributor

Choose a reason for hiding this comment

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

I means that the value of index maybe -1 here, so it will panic.

zap.L().Warn("open file error", zap.Error(err))
return nil, err
}
scanner := bufio.NewScanner(file)
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't that be more concise?

	for lineIndex := int64(0); scanner.Scan() && lineIndex < offset+limit; lineIndex++ {
		if lineIndex >= offset {
			res = append(res, scanner.Text())
		}
	}

Comment on lines 61 to 65
defer func() {
*config.NebulaClientSettings.Connection.Address = address
*config.NebulaClientSettings.Connection.User = user
*config.NebulaClientSettings.Connection.Password = password
}()
Copy link
Contributor

Choose a reason for hiding this comment

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

It's need defer? It's already at the end of func.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If err happen,it also need to do

Copy link
Contributor

@veezhang veezhang left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@veezhang veezhang left a comment

Choose a reason for hiding this comment

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

LGTM

@veezhang veezhang merged commit 8336460 into vesoft-inc:v3.3.0-dev Mar 16, 2022
hetao92 pushed a commit to hetao92/nebula-studio that referenced this pull request Mar 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants