-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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] Convert TransferStatus from enum to class #38374
Conversation
API change check APIView has identified API level changes in this PR and created following API reviews. |
/azp run net - storage - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
sdk/storage/Azure.Storage.DataMovement/src/DataTransferStatus.cs
Outdated
Show resolved
Hide resolved
sdk/storage/Azure.Storage.DataMovement/src/Shared/DataTransferInternalState.cs
Show resolved
Hide resolved
sdk/storage/Azure.Storage.DataMovement/src/Shared/DataTransferStatusInternal.cs
Show resolved
Hide resolved
sdk/storage/Azure.Storage.DataMovement/src/DataTransferStatus.cs
Outdated
Show resolved
Hide resolved
sdk/storage/Azure.Storage.DataMovement/src/Shared/DataTransferInternalState.cs
Outdated
Show resolved
Hide resolved
sdk/storage/Azure.Storage.DataMovement/src/TransferJobInternal.cs
Outdated
Show resolved
Hide resolved
sdk/storage/Azure.Storage.DataMovement/src/ServiceToServiceTransferJob.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.
I think there's some naming consistency work to be done but looking good otherwise.
/azp run net - storage - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
sdk/storage/Azure.Storage.DataMovement/src/DataTransferState.cs
Outdated
Show resolved
Hide resolved
sdk/storage/Azure.Storage.DataMovement/src/DataTransferStatus.cs
Outdated
Show resolved
Hide resolved
sdk/storage/Azure.Storage.DataMovement/src/DataTransferStatus.cs
Outdated
Show resolved
Hide resolved
sdk/storage/Azure.Storage.DataMovement/src/DataTransferStatus.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.
It may be good to merge this soon and apply touchups in separate PRs. This one has a pretty wide blast radius in lines changed and makes it hard to review more incremental changes.
I mostly just had nitpicks at this point but the race condition will be important to address.
…zure#38374) * WIP * WIP * WIP * WIP * WIP - tests updated * Finished fixing tests with changes * Cleanup * ExportApi * WIP * Fixes to live tests * PR Comments - part 1 * Export API * PR Comments
Converting DataTransferStatus from enum to class.
Some of these changes are a lot more trivial than others (e.g. checking enum State in the TransferStatus). The PR is definitely bigger than it seems.
Some of the checkpointer work may be changed anyways when the schema is updated again.