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

BlobContainerFS core logic #1227

Merged
merged 25 commits into from
Apr 7, 2023

Conversation

JasonYeMSFT
Copy link
Contributor

@JasonYeMSFT JasonYeMSFT commented Mar 22, 2023

Implements FileSystemProvider for Azure blobs. The new FS provider no longer queries the tree view for the container node and provides a faster readFile/readDirectory experience. It also implements rename for hierarchical namespace containers.

@JasonYeMSFT JasonYeMSFT marked this pull request as ready for review March 27, 2023 20:23
@JasonYeMSFT JasonYeMSFT requested a review from a team as a code owner March 27, 2023 20:23
@alexweininger
Copy link
Member

Got the build to pass! 🎉

@alexweininger
Copy link
Member

alexweininger commented Mar 29, 2023

We'll need to also support attached storage accounts somehow.

I just tested the current fs provider and it works perfectly with attached storage accounts. So we have two options:

  1. Remove old fs provider and support attached accounts in the new provider
  2. Keep old fs provider around solely for attached account support

I'm sorta voting for option 2 since the attached accounts feature may need a larger rework to get it to work well with the new fs provider.

@JasonYeMSFT
Copy link
Contributor Author

I reverted breaking changes to the AzureStorageFS and added code to use it for attached storage accounts.

alexweininger
alexweininger previously approved these changes Mar 31, 2023
Copy link
Member

@alexweininger alexweininger left a comment

Choose a reason for hiding this comment

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

In my testing it works really well. Lets merge this to get CTI testing done on it and then we can see about releasing this to unblock users from this scenario.

We can leave the fancier stuff like integrating this with the tree view for later.

@JasonYeMSFT
Copy link
Contributor Author

Is there anything needed before merging the PR?

@alexweininger
Copy link
Member

Is there anything needed before merging the PR?

Go ahead and merge.

@alexweininger alexweininger merged commit d8e348b into microsoft:main Apr 7, 2023
@microsoft microsoft locked and limited conversation to collaborators May 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants