Skip to content

fix(gcs-target): Fix incorrect upload path to GCS when it has a leading slash - #170

Merged
BYK merged 1 commit into
masterfrom
byk/fix/gcs-upload-path
Jan 20, 2021
Merged

fix(gcs-target): Fix incorrect upload path to GCS when it has a leading slash#170
BYK merged 1 commit into
masterfrom
byk/fix/gcs-upload-path

Conversation

@BYK

@BYK BYK commented Jan 20, 2021

Copy link
Copy Markdown
Member

Ideally we should fix all the Craft configs to not have this leading slash but that's hard and is also a breaking change so we normalize this ourselves like old times. Fixes #169.

…ng slash

Ideally we should fix all the Craft configs to not have this leading slash but that's hard and is also a breaking change so we normalize this ourselves like old times. Fixes #169.
@BYK
BYK requested review from HazAT, jan-auer and tonyo January 20, 2021 12:51
@BYK
BYK enabled auto-merge (squash) January 20, 2021 13:05

@HazAT HazAT left a comment

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.

q3V3Xe3-796x531

@BYK
BYK merged commit 77d70ba into master Jan 20, 2021
@BYK
BYK deleted the byk/fix/gcs-upload-path branch January 20, 2021 13:34
Comment thread src/utils/gcsApi.ts
// like `//your/dir/and/file` instead of `/your/dir/and/file`
// See #169 for more information.
if (pathInBucket[0] === '/') {
pathInBucket = pathInBucket.substring(1);

@tonyo tonyo Jan 20, 2021

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.

What about multiple leading slashes?
Is it perhaps better to do path.posix.normalize on it?

@BYK BYK Jan 20, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Why would we ever have multiple leading slashes? Also path.posix.normalize doesn't remove leading slashes. We can do let pathInBucket = path.normalize(bucketPath.path) above which removes multiple leading slashes but not sure if this is worth it.

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.

bug: GCS upload is broken

3 participants