Review: Approve

> Hey Edso,
> 
> I just pushed a commit adding documentation to the new feature. Let me know 
> if you need anything else or if I need to do anything different on my end as 
> I have never used bzr or launchpad before only git and svn. 

so far no.

>Also I am not super happy with this overall patch because while its small and 
>noninvasive ideally there should be a way to use a local copy of the manifest 
>file and not have to store the remote one on S3 Standard instead of glacier. 
>If you have any advice/ideas how that could be done without a lot of changes I 
>would love to hear it. 

restoring from remote usually assumes you have no local copy. so why would you 
expect a local manifest to be usable with remote volumes?

>Also while looking at this code it does not seem there is any command checking 
>for exclusive storage options. If I where to enter in duplicity with the 
>options '--s3-use-ia --s3-use-rrs --s3-use-onezone-ia' it looks like it would 
>happily take all the options without throwing an error and store everything on 
>Reduced Redundancy since that is the first option checked in the if statement. 
>I have not looked super deep into this so maybe its checked for elsewhere but 
>it stood out when I made the changes I did.

yeah. saw that as well. generally, as these options are excluding each other 
they should go into one switch. preferably like --s3-storage-class=glacier 
using the string that is used by the API as well. if you'd like o tackle that 
youre welcome. keep backward compatibility though. meaning the other parameters 
setting the new var.

thx.. ede/duply.net
-- 
https://code.launchpad.net/~brandon753-ba/duplicity/aws-glacier/+merge/364306
Your team duplicity-team is subscribed to branch lp:duplicity.

_______________________________________________
Mailing list: https://launchpad.net/~duplicity-team
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~duplicity-team
More help   : https://help.launchpad.net/ListHelp

Reply via email to