-
Notifications
You must be signed in to change notification settings - Fork 228
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
support added to remove files; force flag feature added for file share #2
Conversation
e := removeBlobEnumerator(jobPartOrder) | ||
err = e.enumerate(cca.src, cca.recursive, cca.dst, &wg, cca.waitUntilJobCompletion) | ||
lastPartNumber = e.PartNum | ||
case common.EFromTo.FileTrash(): |
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'm not sure if this word is suitable, would better be more friendly.
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'm OK with Trash. Do you want to suggest a different word?
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 remember Microsoft has TSA which could help to scan special words not suitable to use. And I'm not sure if Trash can pass scan. I know Windows use Recycle bin for deleted files. What about using Recycle?
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 don't care too much about this but I'd be OK with BlobRemove/FileRemove. This solidifies the link between Remove and removeEnumerator.
cmd/removeBlobEnumerator.go
Outdated
waitUntilJobCompletion func(jobID common.JobID, wg *sync.WaitGroup)) error { | ||
return addTransfer((*common.CopyJobPartOrderRequest)(e), transfer, wg, waitUntilJobCompletion) | ||
} | ||
|
||
// send the current list of transfer to the STE | ||
func (e *removeEnumerator) dispatchFinalPart() error { | ||
func (e *removeBlobEnumerator) dispatchFinalPart() error { |
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.
Please check the dispatchFinalPart method like File, dispatchFinalPart((*common.CopyJobPartOrderRequest)(e)), this could save codes.
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.
Thanks for the Jeffery's comments, and in favor of the idea.
|
||
|
||
|
||
// Check if source URL is in directory/* expression, and transfer it to remove from directory if the express is directory/*. |
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.
[minor]Please be aware of empty lines above...
|
||
e.addTransfer(common.CopyTransfer{ | ||
Source: f.String(), | ||
SourceSize: fileInfo.Properties.ContentLength}, |
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 see the difference here is removing function doesn't need destination path and LMT. And see your concern about dup-code in your email. This should be similar for both blob and file, and I'm ok with it if you guys feeling cool~
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 wouldn’t be too concerned about saving code; code size is cheap.
Think more about high-level abstractions. There is a concept of “dispatching the final part”. This abstraction exists for all kinds of transfers and so it should (or could) be a 1st-class concept. In Go, a concept is either a callback function or an interface method both of which define a signature that is part of the 1st-class concept.
Now, you can have several implementations of this concept: one for blobs, one for Azure files, one for local files, etc. So, each of these should implement the callback or interface method. If the implementation for multiple of these is near identical, then that is a coincidence; it is not something that is being forced. If you try to share code, then you are forcing similar implementations. If the implementation must change for one, then it will impact the other. However, this may not be desired. If you keep the implementations separate, then nothing is forced and they may change independently over time.
It may be that there are certain routines that are common and these could be shared as a way to reduce duplicated code. If so, this is ine.
-- Jeffrey Richter (watch Architecting Distributed Cloud Apps<https://aka.ms/RichterCloudApps> on YouTube<https://aka.ms/RichterCloudApps> or edX<https://aka.ms/edx-devops200_9x-about>)
From: Fan Jiachen <notifications@github.com>
Sent: Tuesday, May 15, 2018 1:23 AM
To: Azure/azure-storage-azcopy <azure-storage-azcopy@noreply.github.com>
Cc: Subscribed <subscribed@noreply.github.com>
Subject: Re: [Azure/azure-storage-azcopy] support added to remove files; force flag feature added for file share (#2)
@jiacfan commented on this pull request.
________________________________
In cmd/removeBlobEnumerator.go<#2 (comment)>:
waitUntilJobCompletion func(jobID common.JobID, wg *sync.WaitGroup)) error {
return addTransfer((*common.CopyJobPartOrderRequest)(e), transfer, wg, waitUntilJobCompletion)
}
// send the current list of transfer to the STE
…-func (e *removeEnumerator) dispatchFinalPart() error {
+func (e *removeBlobEnumerator) dispatchFinalPart() error {
Please check the dispatchFinalPart method like File, dispatchFinalPart((*common.CopyJobPartOrderRequest)(e)), this could save codes.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub<#2 (review)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ACK0vyMf4gyMmt8fkS6dwPqgvxTJKwe5ks5typB2gaJpZM4T-TXB>.
|
… include/exclude flag for copy command
…iles for all job whose part plan file exists
…e multiple cancel command from user
Support added to remove and force flag for file share.