Support account_host and check blob size attribute exists #341
Conversation
|
@hayesgb Can you please review the pull request? I'm unable to access the build logs, can you provide me access or steps to get the access? Thanks. |
| f"https://{self.account_name}.blob.core.windows.net" | ||
| ) | ||
| elif self.account_name is not None: | ||
| if hasattr(self, 'account_host'): |
There was a problem hiding this comment.
Can you help me understand where this account_host attribute coming from? Is this from a subclass?
I'd rather have the __init__ method accept something like account_host and make it a property on AzureBlobFileSystem. Then we can avoid all the hasattrs.
The interaction with the current account_name will be a bit complicated. Can you share an example of how account_host is being specified? Maybe we just don't allow users to pass both.
There was a problem hiding this comment.
Adlfs is always appending the public blob endpoint. so having the ability to pass the account_host will help in accessing the non-public regions like gov regions. As account_host is not mandatory I was thinking of consumers to pass as required. We can also add it in init and initialize it as None making as not a required property.
| elif ( | ||
| blob_["name"].count("/") == depth | ||
| and blob_["size"] == 0 | ||
| and (hasattr(blob_, "size") and blob_["size"] == 0) |
There was a problem hiding this comment.
Can you share a bit about why the blob_ would have a size attribute? Does the lack of size attribute indicate a "directory" marker?
There was a problem hiding this comment.
Yes Tom, have seen the blob not having size if the blob type is not a file.
…)" This reverts commit 4e29698.
3 participants
Addresses the issue #340