Tom-Newton commented on code in PR #38505:
URL: https://github.com/apache/arrow/pull/38505#discussion_r1386217313
##########
cpp/src/arrow/filesystem/azurefs.cc:
##########
@@ -453,27 +457,137 @@ class ObjectInputFile final : public
io::RandomAccessFile {
class AzureFileSystem::Impl {
public:
io::IOContext io_context_;
- std::shared_ptr<Azure::Storage::Blobs::BlobServiceClient> service_client_;
+ std::shared_ptr<Azure::Storage::Files::DataLake::DataLakeServiceClient>
+ datalake_service_client_;
+ std::unique_ptr<Azure::Storage::Blobs::BlobServiceClient>
blob_service_client_;
AzureOptions options_;
+ internal::HierarchicalNamespaceDetector hierarchical_namespace_;
Review Comment:
I made it separate because I wanted to keep the cached value `enabled_`
private from the rest of `Impl`. I was a bit concerned that people might try to
directly access the cached state without realising that everything should use
the `Enabled()` function.
I think one possibility is to use a non-smart pointer in
`HierarchicalNamespaceDetector` because `HierarchicalNamespaceDetector` will
always be destructed at the same time as `Impl`.
https://stackoverflow.com/questions/7657718/when-to-use-shared-ptr-and-when-to-use-raw-pointers.
I think that should allow us to use a `unique_ptr` for
`datalake_service_client_`. I think this would be my preferred solution. What
do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
