Skip to content

Conversation

mazurov
Copy link
Contributor

@mazurov mazurov commented Oct 5, 2025

The current WebHDFS.ls() implementation does not accept additional parameters, which breaks compatibility with newer versions of JupyterFS (e.g., jupyterfs==1.1.0).

Since we abstract filesystem interfaces, all common methods (such as ls, info, etc.) should accept **kwargs to ensure forward compatibility with upstream fsspec and JupyterFS expectations.

For example, JupyterFS 1.1.0 calls fs.ls(..., refresh=True) internally, which raises:

TypeError: WebHDFS.ls() got an unexpected keyword argument 'refresh'

Reference:
https://github.com/jpmorganchase/jupyter-fs/blob/6fac72f9a047871de48b757c5634317f88b5ac77/jupyterfs/manager/fsspec.py#L188

Proposed fix:

  • Match the signature of the base class, which accepts **kwargs.
  • Update WebHDFS.ls() signature to accept **kwargs as it implemented for other filesystems (e.g. local)

…g **kwargs

The current WebHDFS.ls() implementation does not accept additional parameters, which breaks compatibility with newer versions of JupyterFS (e.g., jupyterfs==1.1.0).

Since we abstract filesystem interfaces, all common methods (such as ls, info, etc.) should accept **kwargs to ensure forward compatibility with upstream fsspec and JupyterFS expectations.

For example, JupyterFS 1.1.0 calls fs.ls(..., refresh=True) internally, which raises:

TypeError: WebHDFS.ls() got an unexpected keyword argument 'refresh'

Reference:
https://github.com/jpmorganchase/jupyter-fs/blob/6fac72f9a047871de48b757c5634317f88b5ac77/jupyterfs/manager/fsspec.py#L188

Proposed fix:
- Update WebHDFS.ls() signature to accept **kwargs as it implemented for other filesystems (e.g. local)
@martindurant
Copy link
Member

martindurant commented Oct 6, 2025

@RRosio : do you think it's reasonable that jupyter-fsspec should be calling ls(.., refresh=True)? Rather than changing the signatures for filesystems as suggested here, it would be easier to use use_listings_cache=False to the constructor, or call .invalidate_cache() before listing.

However, should we not leave it to the user configuration whether they want directory caching or not?

@martindurant
Copy link
Member

@mazurov , notwithstanding my comment above, your change is still worthwhile, because the superclass signature is

def ls(self, path, detail=True, **kwargs)

@mazurov
Copy link
Contributor Author

mazurov commented Oct 6, 2025

@martindurant, I have to change the original objective from support jupyterfs to follow the signature of the base class

@martindurant martindurant changed the title Make WebHDFS ls() method compatible with newer JupyterFS by supporting **kwargs Make WebHDFS ls() method compatible with superclass by supporting **kwargs Oct 6, 2025
@martindurant martindurant merged commit e12aa75 into fsspec:master Oct 6, 2025
10 checks passed
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.

2 participants