-
Notifications
You must be signed in to change notification settings - Fork 4.9k
[Storage][DataMovement] NFS File-to-File Copy Transfer Preservation #49266
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
[Storage][DataMovement] NFS File-to-File Copy Transfer Preservation #49266
Conversation
sdk/storage/Azure.Storage.DataMovement.Files.Shares/src/ShareFileStorageResource.cs
Outdated
Show resolved
Hide resolved
API change check API changes are not detected in this pull request. |
sdk/storage/Azure.Storage.DataMovement.Files.Shares/src/ShareFileStorageResource.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some code quality notes.
There are clearly some important decisions being made but I need to learn more about the feature to properly review them.
sdk/storage/Azure.Storage.DataMovement.Files.Shares/src/DataMovementSharesExtensions.cs
Outdated
Show resolved
Hide resolved
sdk/storage/Azure.Storage.DataMovement.Files.Shares/src/DataMovementSharesExtensions.cs
Outdated
Show resolved
Hide resolved
sdk/storage/Azure.Storage.DataMovement.Files.Shares/src/DataMovementSharesExtensions.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leaving these two comments but didn't comment on everything we discussed.
...zure.Storage.DataMovement.Files.Shares/api/Azure.Storage.DataMovement.Files.Shares.net6.0.cs
Outdated
Show resolved
Hide resolved
sdk/storage/Azure.Storage.DataMovement.Files.Shares/src/DataMovementSharesExtensions.cs
Outdated
Show resolved
Hide resolved
sdk/storage/Azure.Storage.DataMovement.Files.Shares/tests/Shared/DisposingShare.cs
Outdated
Show resolved
Hide resolved
sdk/storage/Azure.Storage.Files.Shares/src/Models/ShareFileProperties.cs
Show resolved
Hide resolved
<PackageReference Include="Azure.Storage.Files.Shares" /> | ||
<!--<ProjectReference Include="$(MSBuildThisFileDirectory)..\..\Azure.Storage.Files.Shares\src\Azure.Storage.Files.Shares.csproj" />--> | ||
<!-- <PackageReference Include="Azure.Storage.Files.Shares" /> --> | ||
<ProjectReference Include="$(MSBuildThisFileDirectory)..\..\Azure.Storage.Files.Shares\src\Azure.Storage.Files.Shares.csproj" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a github issue made to track to ensure that the Azure.Storage.Files.Storage package gets released first before DMLib (or at the same time). IIRC the base storage SDK packages are released in a separate pipeline than DataMovement now. So a github issue may need to be made to remember to revert this change back before release.
This is only for NFS File-to-File copy transfer. Future works will include Directory-Directory Copy and Uploads and Downloads.
"For NFS service-to-service copy, we need to not set the SMB-specific properties, set the NFS-specific properties, and preserve permissions based on ShareFileStorageResourceOptions.FilePermission."