New PR to enhance compatibility with fsspec and initial implementation of ADL Gen2 #6

hayesgb · 2019-09-11T00:45:34Z

Implementation currently works as follows:

ADL Gen1 using serviceprincipalcredentials with Azure. Requires passing credentials and store_name, with the storage_options parameter in a dask read call. Do not include but store_name in the filepath. When writing to the datalake though, store_name is required to be included in the filepath. Just found this bug and intend to fix it by this weekend, if not before.
ADL Gen2 - Started work on an AzureBlobFileSystem and corresponding AzureBlobFile class. These also authenticate using serviceprincipal credentials. Reading from the datalake seems to be working fine, but the _upload_chunk() method needs additional work.

…ce from AbstractFileSystem and AzureDLFilesystem

Use ms class inheritance

… between files and directoryes

…e.py. This handles the case when the remote file is small enough to be read at once, rather than caching the file. This also replicates the pattern used in the existing implementation of the HTTPFile.

TomAugspurger

Gave an initial pass and things look reasonable. I'm curious about a few things

Where does this live long-term? Is dask/adlfs the right repo? We can easily migrate things
How do we move forward? I think it'd be valuable to get an initial release of this out there and rapidly make new releases as we iterate.

.gitmodules

setup.py

TomAugspurger · 2019-11-11T17:51:08Z

setup.py

+setup(name='adlfs',
+      version='0.1.0',
+      description='Access Azure Datalake Gen1 with fsspec and dask',
+      url='https://github.com/hayesgb/adlfs/',


@martindurant Where's a good long-term home for this? gcsfs & s3fs are both in the Dask org.

This repo is dask/dask-adlfs, which would be fine with me, but also happy to drop the "dask-" part. I should note that MS's library is not called adlfs, even though that's how we referred to it when it was being written and some places within the code still mention that.

For the sake of consistency, it should probably live in the dask org, and I'm fine with that. As far as naming goes, there has been mention of a possible conflict with the Azure library, but as martin mentions above, the MS library isn't called adlfs anywhere I've seen, and that name is consistent with the other cloud libraries.

@TomAugspurger -- Saw work has started on azure-pipeline. I've added you as a maintainer on PyPi.

TomAugspurger · 2019-11-11T17:51:46Z

adlfs/core.py

+
+
+class AzureDatalakeFileSystem(AbstractFileSystem):
+


Style: remove this blank lines.

Recommend flake in the ci, but only after this PR goes in. Can also black, since it's taken over elsewhere.

TomAugspurger · 2019-11-11T17:52:02Z

adlfs/core.py

+    Examples
+    _________
+    >>> adl = AzureDatalakeFileSystem(tenant_id="xxxx", client_id="xxxx", 
+                                    client_secret="xxxx"


Nitpick: these continuation lines should start with ...

TomAugspurger · 2019-11-11T17:53:16Z

adlfs/core.py

+            'client_secret': CLIENT_SECRET
+        })
+
+    Parameters


Typically Parameters comes firs.

Moved parameters above examples.

TomAugspurger · 2019-11-11T17:53:43Z

adlfs/core.py

+        The username or serivceprincipal id
+    client_secret: string
+        The access key
+    store_name: string (None)


Replace None with "optional".

TomAugspurger · 2019-11-11T17:55:09Z

adlfs/core.py

+    def size(self, path):
+        return self.info(path)['length']
+
+    def __getstate__(self):


Is this (still) necessary? I know there's been some upstream changes around serialization.

Probably can replace with simpler reduce, or just use superclass. I'd bet the latter works fine.

TomAugspurger · 2019-11-11T17:56:13Z

adlfs/core.py

+
+
+class AzureDatalakeFileSystem(AbstractFileSystem):
+


Does this need a protocol=?

Yes please, "adl". To some extent, this is implied by including the class in the fsspec registry/ known implementations, but good to be explicit.

Not sure what to call the blob version, though!

Azure datalake Gen2 calls out abfs[s] in the hdfs specification, and blob shares multiprotocol access with Gen2 datalake. OK with protocol = "abfs"?

Perhaps abs for "azure blog storage"?

Agree. Many other file systems name themselves with "fs", but it doesn't appear in the protocol name.

TomAugspurger · 2019-11-11T18:05:07Z

adlfs/core.py

+        logging.debug(f'kwargs are:  {out}')
+        return out
+
+    @classmethod


I wonder how much of this can be placed in a base class that's shared by the two?

Is your question about the instantiation or the staticmethods?

Azure Datalake Gen1 doesn't implement access through an access key like is available for Blob storage.

Sorry, this was a general comment, not directed at this particular line.

…tyle guides

… as fix to bug reading on Datalake Gen1

Resolve suggestions

…ents

TomAugspurger · 2019-11-14T15:33:29Z

@hayesgb just let us know when you're ready here, and I think we'll merge this in.

AlbertDeFusco · 2019-11-14T20:31:45Z

No to throw a wrench into things, but the BlockBlobService function has an argument to utilize the Azurite emulator called is_emulator. When set to true it connects to 127.0.0.1:10000. It might be useful to support that.

https://github.com/hayesgb/adlfs/blob/master/adlfs/core.py#L264

Also, if the Azurite emulator is running on a remote host I've gotten this invocation to work. Again, it might be nice to support some of these arguments in the storage_options.

bbs = BlockBlobService(account_name='devstoreaccount1', account_key='Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==', custom_domain='http://<hostname>:<port>/devstoreaccount1')

I might just make a PR later after you merge this one.

martindurant · 2019-11-14T20:34:12Z

Are you otherwise having a pleasant experience with this code, @AlbertDeFusco ?

AlbertDeFusco · 2019-11-14T20:36:19Z

I haven't tested it personally, but I intend to do it soon. I spent today figuring out how BlockBlobService connected to azurite (locally and remote). I may build this from source and give it a try soon.

AlbertDeFusco · 2019-11-14T20:49:53Z

Edit: I intend to fork this and try it with my Azurite docker container

martindurant · 2019-11-15T14:48:22Z

fsspec/filesystem_spec#205

JoranDox · 2019-11-15T15:22:44Z

@hayesgb in the issue martin linked it's broken again with the latest fsspec, but for the record, I got intake.upload (and the corresponding intake catalog.entry.read) working using:

adlfs (data lake gen 1, regular text file)
fsspec
intake
(pip freeze in linked issue)

if I can provide more useful info just let me know

AlbertDeFusco · 2019-11-15T15:23:38Z

With a change to support custom_domain I have confirmed that this works with the Azurite docker container. I earlier used the azure-storage-blob package to upload the file.

import dask.dataframe as dd
from fsspec.registry import known_implementations
known_implementations['abfs'] = {'class': 'adlfs.AzureBlobFileSystem'}

STORAGE_OPTIONS = {
    'custom_domain':'http://<hostname>:<port>/devstoreaccount1',
    'account_name':'devstoreaccount1',
    'account_key':'Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw=='
}

ddf = dd.read_csv('abfs://data/autompg.csv', storage_options=STORAGE_OPTIONS)
print(ddf.groupby('origin')['mpg'].mean().compute())
    origin
    Asia      30.450633
    Europe    27.602941
    US        20.033469
    Name: mpg, dtype: float64

And fs operations work (including put)

import fsspec

fs = fsspec.filesystem('abfs', container_name='data', **STORAGE_OPTIONS)
print(fs.ls(''))
    ['autompg.csv']

with fs.open('autompg.csv', 'rt') as f:
    d = f.readlines()
print(d[:3])
    ['mpg,cyl,displ,hp,weight,accel,yr,origin,name\n',
     '18.0,8,307.0,130,3504,12.0,70,US,chevrolet chevelle malibu\n',
     '15.0,8,350.0,165,3693,11.5,70,US,buick skylark 320\n']

martindurant · 2019-11-15T15:27:59Z

Note: to add "abs" (better than abfs, I think we decided) and "adl" protocols to the known_implementations in fsspec as soon as this is merged. A release of this repo on pypi and conda-forge ought to follow that shortly.

AlbertDeFusco · 2019-11-15T15:30:14Z

There are some issues with the Quickstart guide in the readme. It should use AzureBlobFileSystem for abs (née abfs), not AzureDatalakeFileSystem.

TomAugspurger · 2019-11-15T18:58:30Z

I'm going to push on this for the next few hours, so that we can resolve fsspec/filesystem_spec#205. Hope that's OK @hayesgb.

TomAugspurger · 2019-11-15T19:05:45Z

requirements.txt

@@ -0,0 +1,4 @@
+azure-datalake-store>=0.0.46,<0.1


Is pinning to <.1 necessary? I'm not familiar with azure's version policy.

Probably not. I don't think they will do much additional development on the azure-datalake-store library, since its for the Gen1 datalake. However, be aware that they're actively working on the storage library, so it should be pinned to <3.0

TomAugspurger · 2019-11-15T19:08:51Z

Do we have a testing strategy in mind? https://github.com/Azure/azure-data-lake-store-python/tree/master/tests uses vcr :/

martindurant · 2019-11-15T19:14:05Z

^ see @AlbertDeFusco 's comments on trying with azurite, which is a docker-based azure storage emulator

martindurant · 2019-11-15T19:14:32Z

https://github.com/Azure/Azurite

TomAugspurger · 2019-11-15T19:16:36Z

Thanks!

AlbertDeFusco · 2019-11-15T19:56:24Z

My fork has support for is_emulated, just set that to true in storage_options after running the azurite docker image

https://github.com/albertdefusco/adlfs

martindurant · 2019-11-15T19:59:00Z

@AlbertDeFusco , please PR to here after this PR is merged, and would appreciate if you have a go at starting the docker image as a fixture and running a basic tests

hayesgb and others added 30 commits February 18, 2019 16:03

Added method to get_pyarrow_filesystem and deleted .gitignore folder

5e735db

Updated class name from ADLFSWrapper to AzureDatalakeFilesystemWrapper

c7a124e

Started to add fsspec AbstractFilesystem

43a63f9

Cloned in fsspec

4800265

Adding mocks for tests

c6d6a2c

Contininued to work on patching

cb1795d

Changed repo name'

a17d83f

ongoing work

6fec6c2

set up multiple inheritance

9e13017

basically works with AzureDatalakeFileSystem using multiple inheritan…

1645044

…ce from AbstractFileSystem and AzureDLFilesystem

Removed mkdir method in AzureDatalakeFileSystem

f705483

Removed mkdir method

3b4ee46

Merge pull request #1 from hayesgb/use_ms_class_inheritance

b0a9260

Use ms class inheritance

Adding files for PyPi

04aaf82

Updated .gitignore for dist/* files

9f21e6c

Merge pull request #2 from hayesgb/use_ms_class_inheritance

465e98d

Use ms class inheritance

Updated readme to reflect pip install method

098fa11

Starting work on abfs[s] filesystem

d5b4bf3

started work on headers and basic list request

e3022d7

core.py ls method started

ed0edae

Added ability to return multiple and nested files, and to distinguish…

9eb18f7

… between files and directoryes

Refined ls method and started on glob method

7bdadd5

progress toward implementing glob

fa30394

started on azureblobfile fetch_all method

7b71740

Added _fetch_all() method to AzureBlobFiles and AllBytes class to cor…

7243fe5

…e.py. This handles the case when the remote file is small enough to be read at once, rather than caching the file. This also replicates the pattern used in the existing implementation of the HTTPFile.

Removed spurious print statements

05aae1a

Implemented PUT operation

1848eb1

Deleted writefile.csv

979ad42

deleted nested subfolder

e1f6058

updated .gitignore

2629d45

martindurant mentioned this pull request Nov 1, 2019

Adapt to fsspec? manish/dask-azureblobfs#73

Open

TomAugspurger reviewed Nov 11, 2019

View reviewed changes

hayesgb added 9 commits November 12, 2019 06:07

Removed filesystem-spec submodule

b209db5

Ran black over core.py, setup.py, and tests.py

c345005

Moved documentation in both the filesystem classes for adherence to s…

31a10b7

…tyle guides

Added protocol='adl' to AzureDatalakeFilesystem class

3837510

Added continuation lines and formating to docs

2e40af5

Added **kwargs to ADLFS _open

a909345

Added **kwarg arguments to inherited methods and pinned fsspec <0.6.0…

d6020b2

… as fix to bug reading on Datalake Gen1

Merge pull request #17 from hayesgb/resolve_suggestions

2335d12

Resolve suggestions

Added long_description_content_type to setup.py to meet pypi requirem…

0e0308e

…ents

TomAugspurger reviewed Nov 15, 2019

View reviewed changes

TomAugspurger merged commit 065b5ba into fsspec:master Nov 15, 2019

New PR to enhance compatibility with fsspec and initial implementation of ADL Gen2 #6

New PR to enhance compatibility with fsspec and initial implementation of ADL Gen2 #6

Conversation

hayesgb commented Sep 11, 2019

TomAugspurger left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomAugspurger commented Nov 14, 2019

AlbertDeFusco commented Nov 14, 2019

martindurant commented Nov 14, 2019

AlbertDeFusco commented Nov 14, 2019

AlbertDeFusco commented Nov 14, 2019

martindurant commented Nov 15, 2019

JoranDox commented Nov 15, 2019

AlbertDeFusco commented Nov 15, 2019 • edited Loading

martindurant commented Nov 15, 2019

AlbertDeFusco commented Nov 15, 2019

TomAugspurger commented Nov 15, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomAugspurger commented Nov 15, 2019

martindurant commented Nov 15, 2019

martindurant commented Nov 15, 2019

TomAugspurger commented Nov 15, 2019

AlbertDeFusco commented Nov 15, 2019

martindurant commented Nov 15, 2019

AlbertDeFusco commented Nov 15, 2019 •

edited

Loading