ADLS Gen2 API Implementation#8280
Conversation
xiafu-msft
requested review from
annatisch,
rakshith91 and
zezha-msft
as code owners
|
Can one of the admins verify this patch? |
There was a problem hiding this comment.
dfs should not be needed here, right? No changes should happen to the blob package
There was a problem hiding this comment.
I think this will be used by from_connection_string, however probably secondary is not necessary?
sdk/storage/azure-storage-file-datalake/azure/storage/file/datalake/mypy.ini
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
/samples/test_datalake_authentication_samples.py [](start = 30, length = 48)
wait, these don't exist yet.
There was a problem hiding this comment.
../samples/test_datalake_service_samples.py [](start = 32, length = 43)
rm these non-existing sample references?
There was a problem hiding this comment.
Can we leave it there and add later?
There was a problem hiding this comment.
wouldn't they screw with the doc tool? since it's pointing to non-existing files
if it works fine,then I'm ok with keeping them
In reply to: 340894009 [](ancestors = 340894009)
There was a problem hiding this comment.
get_user_delegation_key [](start = 8, length = 23)
add test
There was a problem hiding this comment.
def process_storage_error(storage_error): [](start = 1, length = 42)
what's happening here?
There was a problem hiding this comment.
want to deserialize error from adls but haven't got time to add
There was a problem hiding this comment.
although [](start = 57, length = 8)
minor: change although to "even if"
There was a problem hiding this comment.
secondary [](start = 21, length = 9)
secondary is not supported for dfs
There was a problem hiding this comment.
SECONDARY [](start = 4, length = 9)
DFS doesn't support secondary
There was a problem hiding this comment.
fil [](start = 7, length = 4)
some comments would be nice!
...orage/azure-storage-file-datalake/azure/storage/file/datalake/_data_lake_directory_client.py
Outdated
Show resolved
Hide resolved
...orage/azure-storage-file-datalake/azure/storage/file/datalake/_data_lake_directory_client.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
create_file [](start = 8, length = 11)
Should delete_file be added too for consistency?
There was a problem hiding this comment.
not relevant for DFS, can clean up later
sdk/storage/azure-storage-file-datalake/tests/test_directory.py
Outdated
Show resolved
Hide resolved
sdk/storage/azure-storage-file-datalake/tests/test_directory.py
Outdated
Show resolved
Hide resolved
|
Approved with suggestions. Double check that every API is covered by at least one test. |
There was a problem hiding this comment.
should we change this in fileshare and queue too?
...orage/azure-storage-file-datalake/azure/storage/file/datalake/_data_lake_directory_client.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
We've since moved the blob samples from tests/ to their own samples/ folder
...orage/azure-storage-file-datalake/azure/storage/file/datalake/_data_lake_directory_client.py
Outdated
Show resolved
Hide resolved
sdk/storage/azure-storage-file-datalake/azure/storage/file/datalake/_data_lake_lease.py
Outdated
Show resolved
Hide resolved
...orage/azure-storage-file-datalake/azure/storage/file/datalake/_data_lake_directory_client.py
Outdated
Show resolved
Hide resolved
...orage/azure-storage-file-datalake/azure/storage/file/datalake/_data_lake_directory_client.py
Outdated
Show resolved
Hide resolved
xiafu-msft
force-pushed
the
feature/adls
branch
from
0341e86 to
bc6575e
Compare
| Download a file from your file system. | ||
|
|
||
| ```python | ||
| from azure.storage.file.datalake import DataLakeFileClient |
There was a problem hiding this comment.
Did we decide to go with file.datalake or filedatalake?
.docsettings.yml
Outdated
| - ['sdk/storage/azure-storage-blob/swagger/README.md', '#4554'] | ||
|
|
||
| - ['sdk/storage/azure-storage-file-datalake/swagger/README.md', '#4554'] | ||
| - |
There was a problem hiding this comment.
Remove this extra minus sign and Doc Warden should be happier. @scbedd (saw an error here: https://dev.azure.com/azure-sdk/public/_build/results?buildId=181603&view=logs&j=b70e5e73-bbb6-5567-0939-8415943fadb9&t=9069f0a2-bd31-51b1-8b5c-08355cf80229&l=84)
There was a problem hiding this comment.
Just previously there was also a minus sign, it didn't fail the readme test, weird 😄
6 participants
No description provided.