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

[C++][FS][Azure] Implement DeleteFile() for flat-namespace storage accounts #40074

Closed
felipecrv opened this issue Feb 14, 2024 · 3 comments · Fixed by #40075
Closed

[C++][FS][Azure] Implement DeleteFile() for flat-namespace storage accounts #40074

felipecrv opened this issue Feb 14, 2024 · 3 comments · Fixed by #40075

Comments

@felipecrv
Copy link
Contributor

felipecrv commented Feb 14, 2024

Describe the enhancement requested

AzureFileSystem::DeleteFile is implemented for storage accounts with Hierarchical Namespace Support enabled (GH-38703), but not for storage accounts that can use only the Blobs API and represent directories with empty directory marker blobs.

This is a child of GH-18014.

Component(s)

C++

@felipecrv felipecrv changed the title [C++][FS][Azure] Implement DeleteFile for flat-namespace storage accounts [C++][FS][Azure] Implement DeleteFile() for flat-namespace storage accounts Feb 14, 2024
felipecrv added a commit that referenced this issue Feb 21, 2024
…e storage accounts (#40075)

### Rationale for this change

It was not implemented yet.

### What changes are included in this PR?

 - An implementation of `DeleteFile()` that is specialized to storage accounts that don't have HNS support enabled
 - This fixes a semantic issue: deleting a file should not delete the parent directory when the file deleted was the last one
 - Increased test coverage
 - Fix of a bug in the version that deletes files in HNS-enabled accounts (we shouldn't let `DeleteFile` delete directories even if they are empty)

### Are these changes tested?

Yes. Tests were re-written and moved to `TestAzureFileSystemOnAllScenarios`.
* Closes: #40074

Lead-authored-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
Co-authored-by: jerry.adair <Jerry.Adair@sas.com>
Signed-off-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
@felipecrv felipecrv added this to the 16.0.0 milestone Feb 21, 2024
@av8or1
Copy link
Contributor

av8or1 commented Feb 23, 2024

felipecrv - Have you begun work on this one?

@felipecrv
Copy link
Contributor Author

@av8or1 yes, it was closed after the merge of #40075. I added you as co-author in the commit BTW.

@av8or1
Copy link
Contributor

av8or1 commented Feb 23, 2024

Apologies, I didn't include the link! This is the one I was referring to, the move in the flat-namespace storage accounts:
40025

zanmato1984 pushed a commit to zanmato1984/arrow that referenced this issue Feb 28, 2024
…mespace storage accounts (apache#40075)

### Rationale for this change

It was not implemented yet.

### What changes are included in this PR?

 - An implementation of `DeleteFile()` that is specialized to storage accounts that don't have HNS support enabled
 - This fixes a semantic issue: deleting a file should not delete the parent directory when the file deleted was the last one
 - Increased test coverage
 - Fix of a bug in the version that deletes files in HNS-enabled accounts (we shouldn't let `DeleteFile` delete directories even if they are empty)

### Are these changes tested?

Yes. Tests were re-written and moved to `TestAzureFileSystemOnAllScenarios`.
* Closes: apache#40074

Lead-authored-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
Co-authored-by: jerry.adair <Jerry.Adair@sas.com>
Signed-off-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
thisisnic pushed a commit to thisisnic/arrow that referenced this issue Mar 8, 2024
…mespace storage accounts (apache#40075)

### Rationale for this change

It was not implemented yet.

### What changes are included in this PR?

 - An implementation of `DeleteFile()` that is specialized to storage accounts that don't have HNS support enabled
 - This fixes a semantic issue: deleting a file should not delete the parent directory when the file deleted was the last one
 - Increased test coverage
 - Fix of a bug in the version that deletes files in HNS-enabled accounts (we shouldn't let `DeleteFile` delete directories even if they are empty)

### Are these changes tested?

Yes. Tests were re-written and moved to `TestAzureFileSystemOnAllScenarios`.
* Closes: apache#40074

Lead-authored-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
Co-authored-by: jerry.adair <Jerry.Adair@sas.com>
Signed-off-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants