Skip to content

*: generalize and link to the external storage docs from Lightning#5528

Merged
ti-chi-bot merged 9 commits into
pingcap:masterfrom
kennytm:expand-storage-params-to-lightning-dumpling
Mar 15, 2021
Merged

*: generalize and link to the external storage docs from Lightning#5528
ti-chi-bot merged 9 commits into
pingcap:masterfrom
kennytm:expand-storage-params-to-lightning-dumpling

Conversation

@kennytm
Copy link
Copy Markdown
Contributor

@kennytm kennytm commented Feb 18, 2021

What is changed, added or deleted? (Required)

Fix pingcap/docs#4445.

Generalize the "BR storage" page to "external storage", and talk about all associated migration services.

Link back to this page from their documents.

Which TiDB version(s) do your changes apply to? (Required)

  • master (the latest development version)
  • v5.0 (TiDB 5.0 versions)
  • v4.0 (TiDB 4.0 versions)
  • v3.1 (TiDB 3.1 versions)
  • v3.0 (TiDB 3.0 versions)
  • v2.1 (TiDB 2.1 versions)

What is the related PR or file link(s)?

Do your changes match any of the following descriptions?

  • Delete files
  • Change aliases
  • Need modification after applied to another branch
  • Might cause conflicts after applied to another branch

@kennytm
Copy link
Copy Markdown
Contributor Author

kennytm commented Feb 18, 2021

/cc @TomShawn

@ti-srebot ti-srebot requested a review from TomShawn February 18, 2021 08:16
@TomShawn TomShawn self-assigned this Feb 18, 2021
@TomShawn TomShawn added needs-cherry-pick-4.0 size/large Changes of a large size. status/PTAL This PR is ready for reviewing. translation/doing This PR’s assignee is translating this PR. labels Feb 18, 2021
@TomShawn TomShawn requested a review from glorv February 19, 2021 13:09
@ti-srebot
Copy link
Copy Markdown
Contributor

@overvenus, @glorv, @3pointer, @TomShawn, PTAL.

Comment thread br/backup-and-restore-storages.md
@ti-srebot
Copy link
Copy Markdown
Contributor

@glorv, @overvenus, @3pointer, @TomShawn, PTAL.

@kennytm kennytm force-pushed the expand-storage-params-to-lightning-dumpling branch from 4cab887 to dad543e Compare February 22, 2021 11:32
Comment thread br/backup-and-restore-storages.md Outdated
@glorv
Copy link
Copy Markdown
Contributor

glorv commented Feb 23, 2021

LGTM

@ti-srebot
Copy link
Copy Markdown
Contributor

@glorv, Thanks for your review. The bot only counts LGTMs from Reviewers and higher roles, but you're still welcome to leave your comments. See the corresponding SIG page for more information. Related SIG: docs(slack).

@ti-srebot
Copy link
Copy Markdown
Contributor

@overvenus, @3pointer, @TomShawn, PTAL.

@overvenus
Copy link
Copy Markdown
Member

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Feb 25, 2021
@ti-srebot
Copy link
Copy Markdown
Contributor

@glorv, @overvenus, @3pointer, @TomShawn, PTAL.

@ti-srebot
Copy link
Copy Markdown
Contributor

@3pointer, @TomShawn, PTAL.

---
title: BR 存储
summary: 了解 BR 中所用存储服务的 URL 格式。
title: 外部存储
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  • 外部存储 is not only for BR. So we need to change the file name (like tools-remote-storage.md) and put the file out of BR directory.
  • We also need to move its position in the TOC menu. @kissmydb @overvenus @IANTHEREAL Any good idea?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

For tools there's a bunch of these "shared language" description needing such position. Table-filter is another instance.

Copy link
Copy Markdown
Contributor

@TomShawn TomShawn Mar 1, 2021

Choose a reason for hiding this comment

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

  • But in this case, the BR-specific file name is clearly not suitable and can be misleading.

  • For the file position, what about /tools-external-storage.md?

  • For TOC, what about
    image

  • For the table filter case, put it under TiDB 生态工具 LGTM. But it also needs opinion from others.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It belongs to BR, so br/backup-and-restore-storages.md LGTM.

Copy link
Copy Markdown
Contributor

@IANTHEREAL IANTHEREAL Mar 3, 2021

Choose a reason for hiding this comment

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

I agree with @TomShawn, Thanks for the user's understanding perspective.

For 外部存储 and Table-filter

  • the similarity is that the tools ecosystem uses the same code implementation(table-filter and external storage). From the usage perspective, before the implementation is unified (usage, configuration, naming, etc.), it is better to put the introduction in each tool separately; otherwise, we can consider building a special manual for them.
  • the external storage focuses on the external ecosystem supported by the entire tool ecosystem. From this point, it needs to be introduced separately

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@kennytm PTAL

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Note: we have discussed about this internally, and the consensus is we will perform the rename in a separate PR.

@kennytm
Copy link
Copy Markdown
Contributor Author

kennytm commented Mar 15, 2021

PTAL @TomShawn

@TomShawn
Copy link
Copy Markdown
Contributor

/lgtm

@ti-chi-bot
Copy link
Copy Markdown
Member

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • TomShawn

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Details

Reviewer can indicate their review by writing /lgtm in a comment.
Reviewer can cancel approval by writing /lgtm cancel in a comment.

@ti-chi-bot ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Mar 15, 2021
@TomShawn
Copy link
Copy Markdown
Contributor

Follow-up: Change the file name for 外部存储 and remove it out of /br. @IANTHEREAL will work on its new position in TOC.

@ti-chi-bot ti-chi-bot added the requires-followup This PR requires a follow-up task after being merged. label Mar 15, 2021
@TomShawn
Copy link
Copy Markdown
Contributor

/merge

@ti-chi-bot
Copy link
Copy Markdown
Member

This pull request has been accepted and is ready to merge.

DetailsCommit hash: 9f04c75

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 15, 2021
@ti-chi-bot ti-chi-bot merged commit 2c9622b into pingcap:master Mar 15, 2021
ti-srebot pushed a commit to ti-srebot/docs-cn that referenced this pull request Mar 15, 2021
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Copy Markdown
Contributor

cherry pick to release-4.0 in PR #5741

ti-srebot pushed a commit to ti-srebot/docs-cn that referenced this pull request Mar 15, 2021
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Copy Markdown
Contributor

cherry pick to release-5.0 in PR #5742

@TomShawn
Copy link
Copy Markdown
Contributor

/remove-translation doing
/translation done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

requires-followup This PR requires a follow-up task after being merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. status/PTAL This PR is ready for reviewing. translation/done This PR has been translated from English into Chinese and updated to pingcap/docs-cn in a PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants