Skip to content

fix(service/azfile): create parent dir correctly#3655

Closed
CookiePieWw wants to merge 1 commit intoapache:mainfrom
CookiePieWw:main
Closed

fix(service/azfile): create parent dir correctly#3655
CookiePieWw wants to merge 1 commit intoapache:mainfrom
CookiePieWw:main

Conversation

@CookiePieWw
Copy link
Copy Markdown

resolve #3647.

@CookiePieWw CookiePieWw requested a review from Xuanwo as a code owner November 22, 2023 19:01
@github-actions github-actions Bot added the releases-note/fix The PR fixes a bug or has a title that begins with "fix" label Nov 22, 2023
while p != "/" {
p = get_parent(p);
dirs.push_front(p);
p = get_parent(p);
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.

If input path is /a/b/c, will this be wrong?

Copy link
Copy Markdown
Author

@CookiePieWw CookiePieWw Nov 23, 2023

Choose a reason for hiding this comment

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

Thanks for review.

The initial intent here is to avoid puting "/" into the dirs, but ignoring the usage of the function.

Since the document shows that the Create Directory request requires to format the dir like ...path/to/dir..., the problem may lie in the trailing / of the input path. Is it a workable option to just trim the trailing /?
Got it here that the trailing slash will be auto removed. I'll try to figure the root cause out later.

Still I'm not sure the meaning of normalize the root from the issue since I haven't found something wrong about it. Could you give me a hint?

Thanks for your patience.

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.

Still I'm not sure the meaning of normalize the root from the issue since I haven't found something wrong about it. Could you give me a hint?

normalize root is about the code here:

https://github.com/apache/incubator-opendal/blob/0711e6ce97660380628b72a010d092fd29ec9553/core/src/services/azfile/backend.rs#L161

But we have already done that. So it might not be the root cause.

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.

Perhaps we should directly test the azfile service for the error message.

@CookiePieWw
Copy link
Copy Markdown
Author

This issue seems silently fixed by #3651 and #3652

Closed.

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

Labels

releases-note/fix The PR fixes a bug or has a title that begins with "fix"

Projects

None yet

Development

Successfully merging this pull request may close these issues.

services/azfile: azfile doesn't work if root starts with /

2 participants