allow additional arguments to be passed to fsspec via FilesystemConfiguration

[rudolfix]

Background
Our users should be able to pass additional arguments to fsspec filesystems when they are created. A good use case is to enable custom SSL checks on s3:
fsspec/s3fs#342
requires additional argument use_ssl and additional element in client_kwargs (`verify')

Tasks

1. • Extent FilesystemConfiguration so it accepts additional arguments and client arguments ie. via config fields accepting dictionaries
2. • apply arguments in fsspec_from_config properly
3. • Improve the destination factory for filesystem to accept those additional arguments nicely, write docstrings etc.
4. • write tests that make sure that arguments are passed. ie. by providing arguments that break things in predictable way. ie. providing wrong certificate in example above
5. • PR should go to devel branch

[Pipboyguy]

@rudolfix Do we have a specific cert we want to use in our test suite? Does CI have some certificate installed I can use?

[rudolfix]

@Pipboyguy yeah I can create another account on AWS that requires certificate to connect. but maybe we could try this: generate a mock certificate, use it to connect and make sure that we get relevant SSL error which means that certificate is present

WDYT?

[Pipboyguy]

With a self signed cert I guess we'd need a MinIO deployment? If we go with S3 we'd need CloudFront to handle the TLS connection.

@Pipboyguy yeah I can create another account on AWS that requires certificate to connect. but maybe we could try this: generate a mock certificate, use it to connect and make sure that we get relevant SSL error which means that certificate is present

WDYT?

[Pipboyguy]

I'll start with a local MinIO container until further guidance. It's testing it in CI that I'm scratching my head over

[deanja]

Following this issue as I think it will help with implementing git/github fsspec dlt-hub/verified-sources#301 .

The initial case there is to pass org and repo as keyword arguments to filesystem() . Because:

• it keeps the bucket_url and globbing very clean, ie github:// and path/to/**.md.
• org and repo can be included in the fsspec url as github://org:repo@/path/to/file but currently dlt doesn't parse those correctly.

[Pipboyguy]

This default was passed to the filesystem factory each time:

dlt/dlt/common/storages/fsspec_filesystem.py

Line 95 in f648e9d

fs_kwargs["use_listings_cache"] = False # is this default necessary?

I kept it for now, but I wonder whether this shouldn't be an additional argument passed by the user? Should it remain a default configuration?

[deanja]

I looked at the repos at https://github.com/fsspec . It looks like many fsspec implementations - including s3fs, gdrivefs, ftp - do cache the listings (in a DirCache) and they accept fsspec's default of use_listings_cache = True.

I'm pretty new to dlt so I don't know why it is hard-coded to False.

My vote would be to leave the dlt default as False, but give user option to set to True,

This default was passed to the filesystem factory each time:

dlt/dlt/common/storages/fsspec_filesystem.py

Line 95 in f648e9d

fs_kwargs["use_listings_cache"] = False # is this default necessary?

I kept it for now, but I wonder whether this shouldn't be an additional argument passed by the user? Should it remain a default configuration?

[deanja]

@Pipboyguy Is it in your scope to support the additional arguments in the fsspec_filesystem() function too? (in dlt/common/storages/fsspec_filesystem.py) . That function is how the fsspec factory gets called from the dlt filesystem verified source

[rudolfix]

@deanja the reason to get rid of the cache is that most of dlt operations are multiprocess or multithread so even with proper synchronization we were getting "wrong" listings due to cache. so now it is disabled by default.

I also say explicitly that I do not want the cache when doing ls but I'm not sure if just ls is impacted or it uses cache for exists or isfile etc. fsspec is amazing library but the docs are lacking and code is a little bit messy so I prefer to be super strict.

[deanja]

I agree. Caches are mostly frightening! fsspec cache is not accessed in most ETL use cases anyway as you:

• `ls'
• iterate once to get file details, and maybe extract content ( fsspec does not use cache for open)
• done

[Pipboyguy]

@Pipboyguy Is it in your scope to support the additional arguments in the fsspec_filesystem() function too? (in dlt/common/storages/fsspec_filesystem.py) . That function is how the fsspec factory gets called from the dlt filesystem verified source

@deanja One could argue its in this scope. This would be trivial:

def fsspec_filesystem(
    protocol: str,
    credentials: FileSystemCredentials = None,
    kwargs: Optional[DictStrAny] = None,
    client_kwargs: Optional[DictStrAny] = None,
) -> Tuple[AbstractFileSystem, str]:
    return fsspec_from_config(
        FilesystemConfiguration(
            protocol, credentials, kwargs=kwargs, client_kwargs=client_kwargs
        )
    )