Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

gcs: fix wrong path of gcs prefix #647

Merged
merged 4 commits into from
Dec 14, 2020
Merged

Conversation

3pointer
Copy link
Collaborator

@3pointer 3pointer commented Dec 14, 2020

What problem does this PR solve?

backup meta file out of backup directory in gcs storage.

What is changed and how it works?

ensure gcs prefix endwith "/"

Check List

Tests

  • Unit test
  • Manual test (add detailed scripts or steps below)
    before:
    image
    after:
    image

Related changes

  • Need to cherry-pick to the release branch

Release Note

  • Fix the issue that write backup meta file out of backup directory in gcs storage.

@@ -84,9 +85,16 @@ type gcsStorage struct {
bucket *storage.BucketHandle
}

func (s *gcsStorage) objectName(name string) string {
if strings.HasSuffix(s.gcs.Prefix, "/") {
return s.gcs.Prefix + name

Choose a reason for hiding this comment

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

Maybe path.join(s.gcs.Prefix ,name) is better.

@glorv
Copy link
Collaborator

glorv commented Dec 14, 2020

LGTM

@ti-srebot ti-srebot added the status/LGT1 LGTM1 label Dec 14, 2020
Copy link
Member

@overvenus overvenus left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot removed the status/LGT1 LGTM1 label Dec 14, 2020
@ti-srebot ti-srebot added the status/LGT2 LGTM2 label Dec 14, 2020
@3pointer
Copy link
Collaborator Author

/run-integration-test

@kennytm
Copy link
Collaborator

kennytm commented Dec 14, 2020

please also move this to apply to all storage types

prefix := strings.Trim(u.Path, "/")

Copy link
Collaborator

@kennytm kennytm left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added status/LGT3 LGTM3. This PR looks very good to our bot. and removed status/LGT2 LGTM2 labels Dec 14, 2020
@3pointer 3pointer merged commit 948c362 into pingcap:master Dec 14, 2020
ti-srebot pushed a commit to ti-srebot/br that referenced this pull request Dec 14, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #648

3pointer added a commit that referenced this pull request Dec 14, 2020
* cherry pick #647 to release-4.0

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

* fix conflict

Co-authored-by: 3pointer <luancheng@pingcap.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs-cherry-pick-release-4.0 status/LGT3 LGTM3. This PR looks very good to our bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants