search

daknob / TorPaste

A Pastebin for Tor
MIT License
15 stars 7 forks source link

Implement Azure Storage backend #57

Closed laura-barluzzi closed 7 years ago

laura-barluzzi commented 7 years ago

This pull request adds a storage backend based on Azure Blob Storage. See #15

Azure Blob Storage was used instead of Azure Table Storage to implement the backend because the latter has a 1MB limit on column size (source) which means that the backend wouldn't be able to support large pastes. Azure Blob Storage already supports custom metadata associated with blobs so we can fully implement the storage interface in terms of Azure Blob Storage.

Pair-programmed with @c-w

daknob commented 7 years ago

Hello and thanks for that great PR! I have a question, which may not be a good one: from what I can tell, you convert all AzureExceptions to ErrorExceptions. This is done using the same Exception message. Is there a chance this will show the user private information?

In general I'd like backends to return Exceptions with messages that are ready for the user to see. So it's better to say a "Backend Error Occurred" rather than "Azure: bucket X failed write: rate limiting" or something like that.

If you believe this is not the case, and the exceptions raised are user-friendly, I can go ahead and merge it.

Thanks again!

laura-barluzzi commented 7 years ago

Hi @DaKnOb, Thank you for the comment. I totally get your concern and I pushed a commit to improve the error messages. Now, whenever there's an AzureException, the user will see this message: Error communicating with Azure storage service Does this address your point?

daknob commented 7 years ago

Hi, and thanks for getting back to me. I think if you replace the text with Error while communicating with the Azure Storage Service, it will be better, and I can merge this.

Thanks a lot, and sorry for asking for such a simple fix!

laura-barluzzi commented 7 years ago

Hi @DaKnOb, Great to hear you are fine with this solution! Also, don't worry about the requirement -- improving user experience is very important.

For your information the fix is already pushed via the commit makes ErrorException statement user friendly

It should be ready to go :)

daknob commented 7 years ago

Thank you very much for this contribution!