Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update README.md #19

Closed
wants to merge 2 commits into from
Closed

Update README.md #19

wants to merge 2 commits into from

Conversation

Timost
Copy link

@Timost Timost commented Nov 26, 2019

Hi !
First of all thank you for working on this much needed azure storage layer.

I think the readme is not correct for the Gen 2 details. I think it should pass the AzureBlobFileSystem class instead of the AzureDatalakeFileSystem.

If that is correct. Here is the PR fixing the readme.

@martindurant
Copy link
Member

Thank you for picking up on this.
After the coming change in fsspec, this step should not be necessary at all, so I expect this PR to be superseded ( cc @TomAugspurger )

(note that the failure is due to the implementation of linting)

@Timost
Copy link
Author

Timost commented Nov 26, 2019

Thanks for the quick reply ! Regarding the upcoming change in fsspec. Do you mean this Issue: fsspec/filesystem_spec#206 ?

@martindurant
Copy link
Member

That is an issue yet to be implemented, and that would work too, but I actually mean directly adding the implementation classes here to fsspec.registry.known_implementations

@Timost
Copy link
Author

Timost commented Nov 26, 2019

Oh ok, I couldn't find the related PR. Are you interested in this PR then ? If yes, do you want me to edit the setup.py file for the linting to pass ?

@martindurant
Copy link
Member

Yes, that PR to fsspec should be made.
Following that, the lines here concerning the fsspec registry should simply be removed, they will not be necessary. Regarding linting, yes, we should just accept what the linter says is the right format (I don't know why it changed).

@hayesgb
Copy link
Collaborator

hayesgb commented Nov 28, 2019

Cross-referencing with this PR.

@@ -23,7 +23,7 @@ To use the Gen2 filesystem:
```
import dask.dataframe as dd
from fsspec.registry import known_implementations
known_implementations['abfs'] = {'class': 'adlfs.AzureDatalakeFileSystem'}
known_implementations['abfs'] = {'class': 'adlfs.AzureBlobFileSystem'}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be needed anymore, right?

@hayesgb
Copy link
Collaborator

hayesgb commented Nov 28, 2019 via email

@Timost
Copy link
Author

Timost commented Dec 4, 2019

Ok I guess when fsspec/filesystem_spec#209 is released, this PR can be closed ? Thank you for adding this support !

@Timost
Copy link
Author

Timost commented Jan 2, 2020

I'm closing this PR. The ffspec known_protocols registry no longer needs patching as adl and abfs are now there in fsspec.

@Timost Timost closed this Jan 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants