Azure / azure-storage-cpplite

Lite version of C++ Client Library for Microsoft Azure Storage
MIT License
25 stars 43 forks source link

We should add sample of `blob_client` and remove sample of `blob_client_wrapper` #68

Closed Jinming-Hu closed 4 years ago

Jinming-Hu commented 4 years ago

The only sample in this project is for blob_client_wrapper, which is designed for a specific user rather than ordinary users. While it misleads most users into using our cpplite sdk the wrong way.

The interface of blob_client_wrapper has a flaw. It returns void rather than a value indicating whether an error has occurred. So the user must check errno everytime after calling a function. This is counter-intuitive and unfriendly, and also not conventional. In Linux, APIs usually return an int, if the value is -1, error information is stored in errno. Windows APIs work in a similar way, if an API returns a false or INVALID_HANDLE_VALUE, error code is stored in GetLastError(). We have another user complaining about this https://github.com/Azure/azure-storage-cpplite/issues/31.

In addition, it is error-prone, because errno is very likely to be overwritten by other C/C++ functions, like printf, std::cout or std::fstream, these stuff are implementation-dependent. We also have users reporting these kind of issues, like https://github.com/Azure/azure-storage-cpplite/issues/47, https://github.com/Azure/azure-storage-cpplite/issues/67.

Jinming-Hu commented 4 years ago

@katmsft I'd like to have your thoughts on this matter. Thanks

katmsft commented 4 years ago

I think it is better we have made it official that blob_client_wrapper is not recommended to be used unless customer cannot tolerate exceptions in their code base. With this said, not only samples, but also documents/comments should be made clear that we do not encourage our customer to use it unless neccessary. There is also a feature gap, where blob_client_wrapper supports concurrent upload, which needs to be ported to blob_client. So in short, I think this is a valid item.

prasanthrgb commented 4 years ago

Hi Guys, Does this issue fix available? Thanks, Prasanth

Jinming-Hu commented 4 years ago

@prasanthrgb Yes, please have a look at this PR #63.

Jinming-Hu commented 4 years ago

Sample is added in 0.3.0 release, and I'll created another issue #73 discussing the concurrent upload/download feature brought up by @katmsft . I'm going to close this issue for now.