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

[docs] Added local file configuration guide for resource center #10264

Merged
merged 13 commits into from
May 30, 2022

Conversation

GavinGYM
Copy link
Contributor

Purpose of the pull request

This pull request added Local File Resource Configuration Guide to the document, updated file path for common.properties, and removed unnecessary tips.

Brief change log

Modified docs/en/guide/resource/configuration.md
Modified docs/zh/guide/resource/configuration.md

Verify this pull request

This pull request is code cleanup without any test coverage.

@SbloodyS SbloodyS added first time contributor First-time contributor improvement make more easy to user or prompt friendly document labels May 27, 2022
docs/docs/en/guide/resource/configuration.md Outdated Show resolved Hide resolved
docs/docs/en/guide/resource/configuration.md Outdated Show resolved Hide resolved
docs/docs/en/guide/resource/configuration.md Outdated Show resolved Hide resolved
docs/docs/zh/guide/resource/configuration.md Outdated Show resolved Hide resolved
docs/docs/zh/guide/resource/configuration.md Outdated Show resolved Hide resolved
SbloodyS
SbloodyS previously approved these changes May 27, 2022
Copy link
Member

@SbloodyS SbloodyS left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for you contribution.

@zhongjiajie Do you have better suggestions?

@SbloodyS SbloodyS added this to the 3.0.0-beta-2 milestone May 27, 2022
@zhongjiajie
Copy link
Member

Hi @GavinGYM could we combine this PR to #10265, I think is better to submit PR together with code and docs instead of separating them WDYT @SbloodyS @QuakeWang

SbloodyS
SbloodyS previously approved these changes May 30, 2022
docs/docs/en/guide/resource/configuration.md Outdated Show resolved Hide resolved

Configure the file in the following paths: `api-server/conf/common.properties` and `worker-server/conf/common.properties`.

- Change 'data.basedir.path' to the local directory path. Please make sure the directory exists and the user who deploy dolphinscheduler have read and write permissions, such as: `data.basedir.path=/tmp/dolphinscheduler`.
Copy link
Member

Choose a reason for hiding this comment

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

Same as the previous comment

Suggested change
- Change 'data.basedir.path' to the local directory path. Please make sure the directory exists and the user who deploy dolphinscheduler have read and write permissions, such as: `data.basedir.path=/tmp/dolphinscheduler`.
- Change `data.basedir.path` to the local directory path. Please make sure the directory exists and the user who deploy dolphinscheduler have read and write permissions, such as: `data.basedir.path=/tmp/dolphinscheduler`.

Copy link
Member

Choose a reason for hiding this comment

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

I think it will auto-create directory if not exists, so maybe we could delete the Please make sure the directory exists

private void initHdfsPath() {
Path path = new Path(RESOURCE_UPLOAD_PATH);
try {
if (!fs.exists(path)) {
fs.mkdirs(path);
}
} catch (Exception e) {
logger.error(e.getMessage(), e);
}
}

Copy link
Member

Choose a reason for hiding this comment

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

FYI @GavinGYM

@zhongjiajie
Copy link
Member

Hi @GavinGYM could we combine this PR to #10265, I think is better to submit PR together with code and docs instead of separating them WDYT @SbloodyS @QuakeWang

I got it, the PR author is not the same, pls ignore this comment

docs/docs/zh/guide/resource/configuration.md Outdated Show resolved Hide resolved
docs/docs/zh/guide/resource/configuration.md Outdated Show resolved Hide resolved
docs/docs/zh/guide/resource/configuration.md Show resolved Hide resolved
docs/docs/en/guide/resource/configuration.md Show resolved Hide resolved
docs/docs/en/guide/resource/configuration.md Outdated Show resolved Hide resolved
Copy link
Member

@zhongjiajie zhongjiajie left a comment

Choose a reason for hiding this comment

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

LGTM, thank @GavinGYM

@zhongjiajie zhongjiajie merged commit 2294160 into apache:dev May 30, 2022
@zhongjiajie zhongjiajie changed the title [Feature-9385][docs] Added Local File Resource Configuration Guide to the document [docs] Added local file configuration guide for resource center May 30, 2022
@zhongjiajie
Copy link
Member

BTW, capitalizing the first word of a sentence is enough for both PR or issue title @GavinGYM

@GavinGYM
Copy link
Contributor Author

LGTM, thank @GavinGYM

My pleasure.

@GavinGYM
Copy link
Contributor Author

BTW, capitalizing the first word of a sentence is enough for both PR or issue title @GavinGYM

Got it. I'll pay attention next time.

devosend pushed a commit that referenced this pull request Jun 17, 2022
* Added Local File Resource Configuration Guide to the document.
* Removed contents with windows features in the documents and improved expression.
* Specify `the user who deploy dolphinscheduler have read and write permissions` in en and zh docs.

Co-authored-by: xiangzihao <460888207@qq.com>
(cherry picked from commit 2294160)
ITBOX-ITBOY pushed a commit to ITBOX-ITBOY/dolphinscheduler that referenced this pull request Jul 8, 2022
…he#10264)

* Added Local File Resource Configuration Guide to the document.
* Removed contents with windows features in the documents and improved expression.
* Specify `the user who deploy dolphinscheduler have read and write permissions` in en and zh docs.

Co-authored-by: xiangzihao <460888207@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
document first time contributor First-time contributor improvement make more easy to user or prompt friendly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants