Repository: fsspec/filesystem_spec
Pull request #1839 (2025-05-04 14:58:37)

Add support for rm_file in GitHubFileSystem implementation
This PR adds support for the `rm_file` method in the `GitHubFileSystem` implementation of fsspec, enabling file deletion using the GitHub REST API.

The [GitHub API](https://docs.github.com/en/rest/repos/contents?apiVersion=2022-11-28#create-or-update-file-contents) for managing file contents requires:

- A commit message for each action (add/update/delete), similar to editing files via the GitHub web interface.
- The SHA of the target file’s latest version when deleting or updating, to avoid race conditions.

This implementation adheres to those requirements, making rm_file work reliably for GitHub-backed filesystems.

Note: This PR focuses solely on file deletion. I plan to follow up with a separate PR to implement the remaining methods and functionality (e.g., adding file, updating file, etc.) for full `GitHubFileSystem` support.

Fixes https://github.com/fsspec/filesystem_spec/issues/1836


Patch:
diff --git a/fsspec/implementations/cached.py b/fsspec/implementations/cached.py
index bd58ad4..929ec1f 100644
--- a/fsspec/implementations/cached.py
+++ b/fsspec/implementations/cached.py
@@ -335,6 +335,11 @@ class CachingFileSystem(AbstractFileSystem):
             self._metadata.update_file(path, detail)
             logger.debug("Creating local sparse file for %s", path)
 
+        # explicitly submitting the size to the open call will avoid extra
+        # operations when opening. This is particularly relevant
+        # for any file that is read over a network, e.g. S3.
+        size = detail.get("size", None)
+
         # call target filesystems open
         self._mkcache()
         f = self.fs._open(
@@ -344,8 +349,15 @@ class CachingFileSystem(AbstractFileSystem):
             autocommit=autocommit,
             cache_options=cache_options,
             cache_type="none",
+            size=size,
             **kwargs,
         )
+
+        # set size if not already set
+        if size is None:
+            detail["size"] = f.size
+            self._metadata.update_file(path, detail)
+
         if self.compression:
             comp = (
                 infer_compression(path)
diff --git a/fsspec/implementations/dbfs.py b/fsspec/implementations/dbfs.py
index fa71e1a..2c56a42 100644
--- a/fsspec/implementations/dbfs.py
+++ b/fsspec/implementations/dbfs.py
@@ -14,12 +14,13 @@ class DatabricksException(Exception):
     Helper class for exceptions raised in this module.
     """
 
-    def __init__(self, error_code, message):
+    def __init__(self, error_code, message, details=None):
         """Create a new DatabricksException"""
         super().__init__(message)
 
         self.error_code = error_code
         self.message = message
+        self.details = details
 
 
 class DatabricksFileSystem(AbstractFileSystem):
@@ -80,7 +81,7 @@ class DatabricksFileSystem(AbstractFileSystem):
                     raise FileNotFoundError(e.message) from e
 
                 raise
-            files = r["files"]
+            files = r.get("files", [])
             out = [
                 {
                     "name": o["path"],
diff --git a/fsspec/implementations/github.py b/fsspec/implementations/github.py
index b624ca5..ea29721 100644
--- a/fsspec/implementations/github.py
+++ b/fsspec/implementations/github.py
@@ -1,4 +1,5 @@
 import base64
+import re
 
 import requests
 
@@ -265,3 +266,70 @@ class GithubFileSystem(AbstractFileSystem):
             cache_options=cache_options,
             **kwargs,
         )
+
+    def rm(self, path, recursive=False, maxdepth=None, message=None):
+        path = self.expand_path(path, recursive=recursive, maxdepth=maxdepth)
+        for p in reversed(path):
+            self.rm_file(p, message=message)
+
+    def rm_file(self, path, message=None, **kwargs):
+        """
+        Remove a file from a specified branch using a given commit message.
+
+        Since Github DELETE operation requires a branch name, and we can't reliably
+        determine whether the provided SHA refers to a branch, tag, or commit, we
+        assume it's a branch. If it's not, the user will encounter an error when
+        attempting to retrieve the file SHA or delete the file.
+
+        Parameters
+        ----------
+        path: str
+            The file's location relative to the repository root.
+        message: str, optional
+            The commit message for the deletion.
+        """
+
+        if not self.username:
+            raise ValueError("Authentication required")
+
+        path = self._strip_protocol(path)
+
+        # Attempt to get SHA from cache or Github API
+        sha = self._get_sha_from_cache(path)
+        if not sha:
+            url = self.content_url.format(
+                org=self.org, repo=self.repo, path=path.lstrip("/"), sha=self.root
+            )
+            r = requests.get(url, timeout=self.timeout, **self.kw)
+            if r.status_code == 404:
+                raise FileNotFoundError(path)
+            r.raise_for_status()
+            sha = r.json()["sha"]
+
+        # Delete the file
+        delete_url = self.content_url.format(
+            org=self.org, repo=self.repo, path=path, sha=self.root
+        )
+        branch = self.root
+        data = {
+            "message": message or f"Delete {path}",
+            "sha": sha,
+            **({"branch": branch} if branch else {}),
+        }
+
+        r = requests.delete(delete_url, json=data, timeout=self.timeout, **self.kw)
+        error_message = r.json().get("message", "")
+        if re.search(r"Branch .+ not found", error_message):
+            error = "Remove only works when the filesystem is initialised from a branch or default (None)"
+            raise ValueError(error)
+        r.raise_for_status()
+
+        self.invalidate_cache(path)
+
+    def _get_sha_from_cache(self, path):
+        for entries in self.dircache.values():
+            for entry in entries:
+                entry_path = entry.get("name")
+                if entry_path and entry_path == path and "sha" in entry:
+                    return entry["sha"]
+        return None
diff --git a/pyproject.toml b/pyproject.toml
index abe106c..c00e306 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -183,7 +183,7 @@ select = [
     "SIM",
     "SLOT",
     "SIM101",
-    "TCH",
+    "TC",
     "UP",
 ]
 ignore = [


Test patch:
diff --git a/fsspec/implementations/tests/cassettes/test_dbfs/test_dbfs_file_listing.yaml b/fsspec/implementations/tests/cassettes/test_dbfs/test_dbfs_file_listing.yaml
deleted file mode 100644
index 57929bf..0000000
--- a/fsspec/implementations/tests/cassettes/test_dbfs/test_dbfs_file_listing.yaml
+++ /dev/null
@@ -1,112 +0,0 @@
-interactions:
-- request:
-    body: '{"path": "/"}'
-    headers:
-      Accept:
-      - '*/*'
-      Accept-Encoding:
-      - gzip, deflate
-      Connection:
-      - keep-alive
-      Content-Length:
-      - '13'
-      Content-Type:
-      - application/json
-      User-Agent:
-      - python-requests/2.31.0
-      authorization:
-      - DUMMY
-    method: GET
-    uri: https://my_instance.com/api/2.0/dbfs/list
-  response:
-    body:
-      string: !!binary |
-        H4sIAAAAAAAEA6zSSwrCMBAG4LvMumLSpo3JAbyA4Eak1DbiYF8kExeW3t2qC6G46WM3/AwfzKOD
-        K5bGgT510GZ0Aw3b/ZAcqLEGAkCXFmhBk/Um+PSmDp8GNAugagq8Yp4RNnVKWA0pl0woxmMhGGN9
-        8DOPTemHhsngH8QtVIqMsovF/O4279IZWlG0xvlyMVjVNH1InoTD5kMpotHuqWrnaErGkeJyN9K8
-        M3YOxxmLEhGqEfdY4zG+yKwznvsXAAAA//8DAMDPrToDAwAA
-    headers:
-      access-control-allow-headers:
-      - Authorization, X-Databricks-Azure-Workspace-Resource-Id, X-Databricks-Org-Id,
-        Content-Type
-      access-control-allow-origin:
-      - '*'
-      cache-control:
-      - no-cache, no-store, must-revalidate
-      content-encoding:
-      - gzip
-      content-type:
-      - application/json
-      expires:
-      - '0'
-      pragma:
-      - no-cache
-      server:
-      - databricks
-      strict-transport-security:
-      - max-age=31536000; includeSubDomains; preload
-      transfer-encoding:
-      - chunked
-      vary:
-      - Accept-Encoding
-      x-content-type-options:
-      - nosniff
-    status:
-      code: 200
-      message: OK
-- request:
-    body: '{"path": "/"}'
-    headers:
-      Accept:
-      - '*/*'
-      Accept-Encoding:
-      - gzip, deflate
-      Connection:
-      - keep-alive
-      Content-Length:
-      - '13'
-      Content-Type:
-      - application/json
-      User-Agent:
-      - python-requests/2.31.0
-      authorization:
-      - DUMMY
-    method: GET
-    uri: https://my_instance.com/api/2.0/dbfs/list
-  response:
-    body:
-      string: !!binary |
-        H4sIAAAAAAAEA6zSSwrCMBAG4LvMumLSpo3JAbyA4Eak1DbiYF8kExeW3t2qC6G46WM3/AwfzKOD
-        K5bGgT510GZ0Aw3b/ZAcqLEGAkCXFmhBk/Um+PSmDp8GNAugagq8Yp4RNnVKWA0pl0woxmMhGGN9
-        8DOPTemHhsngH8QtVIqMsovF/O4279IZWlG0xvlyMVjVNH1InoTD5kMpotHuqWrnaErGkeJyN9K8
-        M3YOxxmLEhGqEfdY4zG+yKwznvsXAAAA//8DAMDPrToDAwAA
-    headers:
-      access-control-allow-headers:
-      - Authorization, X-Databricks-Azure-Workspace-Resource-Id, X-Databricks-Org-Id,
-        Content-Type
-      access-control-allow-origin:
-      - '*'
-      cache-control:
-      - no-cache, no-store, must-revalidate
-      content-encoding:
-      - gzip
-      content-type:
-      - application/json
-      expires:
-      - '0'
-      pragma:
-      - no-cache
-      server:
-      - databricks
-      strict-transport-security:
-      - max-age=31536000; includeSubDomains; preload
-      transfer-encoding:
-      - chunked
-      vary:
-      - Accept-Encoding
-      x-content-type-options:
-      - nosniff
-    status:
-      code: 200
-      message: OK
-version: 1
diff --git a/fsspec/implementations/tests/cassettes/test_dbfs/test_dbfs_listing.yaml b/fsspec/implementations/tests/cassettes/test_dbfs/test_dbfs_listing.yaml
new file mode 100644
index 0000000..866f32a
--- /dev/null
+++ b/fsspec/implementations/tests/cassettes/test_dbfs/test_dbfs_listing.yaml
@@ -0,0 +1,214 @@
+interactions:
+- request:
+    body: '{"path": "/FileStore/fsspec_dbfs/project_root/empty_dir"}'
+    headers:
+      Accept:
+      - '*/*'
+      Accept-Encoding:
+      - gzip, deflate
+      Connection:
+      - keep-alive
+      Content-Length:
+      - '57'
+      Content-Type:
+      - application/json
+      User-Agent:
+      - python-requests/2.32.3
+      authorization:
+      - DUMMY
+    method: POST
+    uri: https://my_instance.com/api/2.0/dbfs/mkdirs
+  response:
+    body:
+      string: '{}'
+    headers:
+      access-control-allow-headers:
+      - Authorization, X-Databricks-Azure-Workspace-Resource-Id, X-Databricks-Org-Id,
+        Content-Type
+      access-control-allow-origin:
+      - '*'
+      cache-control:
+      - no-cache, no-store, must-revalidate
+      content-length:
+      - '2'
+      content-type:
+      - application/json
+      expires:
+      - '0'
+      pragma:
+      - no-cache
+      server:
+      - databricks
+      strict-transport-security:
+      - max-age=31536000; includeSubDomains; preload
+      x-content-type-options:
+      - nosniff
+      x-request-id:
+      - e71bb75f-b05a-4523-b8bc-ce331812fbf0
+    status:
+      code: 200
+      message: OK
+- request:
+    body: '{"path": "/FileStore/fsspec_dbfs"}'
+    headers:
+      Accept:
+      - '*/*'
+      Accept-Encoding:
+      - gzip, deflate
+      Connection:
+      - keep-alive
+      Content-Length:
+      - '34'
+      Content-Type:
+      - application/json
+      User-Agent:
+      - python-requests/2.32.3
+      authorization:
+      - DUMMY
+    method: GET
+    uri: https://my_instance.com/api/2.0/dbfs/list
+  response:
+    body:
+      string: !!binary |
+        H4sIAAAAAAAEAxzMMQ7CMAwF0Lv8OVKYcwAuwIiQ1SaOMKI4ss3SqncvZX3D29DlzY5y3zCmeKIg
+        X39yCzXO3X1wpTZ3z8P0xTXIVAMJ4tTEUMK+nP4JuayMcklYtEmXOoXoh0KWU/fHfgAAAP//AwCB
+        uORhbAAAAA==
+    headers:
+      access-control-allow-headers:
+      - Authorization, X-Databricks-Azure-Workspace-Resource-Id, X-Databricks-Org-Id,
+        Content-Type
+      access-control-allow-origin:
+      - '*'
+      cache-control:
+      - no-cache, no-store, must-revalidate
+      content-encoding:
+      - gzip
+      content-type:
+      - application/json
+      expires:
+      - '0'
+      pragma:
+      - no-cache
+      server:
+      - databricks
+      strict-transport-security:
+      - max-age=31536000; includeSubDomains; preload
+      transfer-encoding:
+      - chunked
+      vary:
+      - Accept-Encoding
+      x-content-type-options:
+      - nosniff
+      x-request-id:
+      - 6e66514c-c883-49c7-b215-2fecc5ef8222
+    status:
+      code: 200
+      message: OK
+- request:
+    body: '{"path": "/FileStore/fsspec_dbfs/project_root/empty_dir"}'
+    headers:
+      Accept:
+      - '*/*'
+      Accept-Encoding:
+      - gzip, deflate
+      Connection:
+      - keep-alive
+      Content-Length:
+      - '57'
+      Content-Type:
+      - application/json
+      User-Agent:
+      - python-requests/2.32.3
+      authorization:
+      - DUMMY
+    method: GET
+    uri: https://my_instance.com/api/2.0/dbfs/list
+  response:
+    body:
+      string: '{}'
+    headers:
+      access-control-allow-headers:
+      - Authorization, X-Databricks-Azure-Workspace-Resource-Id, X-Databricks-Org-Id,
+        Content-Type
+      access-control-allow-origin:
+      - '*'
+      cache-control:
+      - no-cache, no-store, must-revalidate
+      content-length:
+      - '2'
+      content-type:
+      - application/json
+      expires:
+      - '0'
+      pragma:
+      - no-cache
+      server:
+      - databricks
+      strict-transport-security:
+      - max-age=31536000; includeSubDomains; preload
+      x-content-type-options:
+      - nosniff
+      x-request-id:
+      - 59b8c763-be19-4402-8d37-135a7d6c7aed
+    status:
+      code: 200
+      message: OK
+- request:
+    body: '{"path": "/FileStore/fsspec_dbfs/project_root/missing_directory"}'
+    headers:
+      Accept:
+      - '*/*'
+      Accept-Encoding:
+      - gzip, deflate
+      Connection:
+      - keep-alive
+      Content-Length:
+      - '65'
+      Content-Type:
+      - application/json
+      User-Agent:
+      - python-requests/2.32.3
+      authorization:
+      - DUMMY
+    method: GET
+    uri: https://my_instance.com/api/2.0/dbfs/list
+  response:
+    body:
+      string: !!binary |
+        H4sIAAAAAAAEAzyOwWrDMBBEf0XoXNtx4hqSU6B1IZcY4hQKpQhFWrkqtlfdVUtDyL9XbqCnZdh5
+        M3ORQISkDFqQG3louvb58NCox7bp1L49quZl1x3lnRyBWfezZ4/C+QEEkrCewESks4Afz5EFTiLo
+        +C6Kp+To0gcKxxzAKHtyXATCjwQoQozF6Jn91Kv/kDzVWIjaDyw3rxe5jecwF84n7xH7AXTwnBsc
+        i5vMKZj8AJ9fwHE3OUwBdFPK20TWTjuoa8jK2q6yamGq7ATrZbYuYelW9aLU91ViGOj7b4mOOlHy
+        +nb9BQAA//8DADr+FHAYAQAA
+    headers:
+      access-control-allow-headers:
+      - Authorization, X-Databricks-Azure-Workspace-Resource-Id, X-Databricks-Org-Id,
+        Content-Type
+      access-control-allow-origin:
+      - '*'
+      cache-control:
+      - no-cache, no-store, must-revalidate
+      content-encoding:
+      - gzip
+      content-type:
+      - application/json
+      expires:
+      - '0'
+      pragma:
+      - no-cache
+      server:
+      - databricks
+      strict-transport-security:
+      - max-age=31536000; includeSubDomains; preload
+      transfer-encoding:
+      - chunked
+      vary:
+      - Accept-Encoding
+      x-content-type-options:
+      - nosniff
+      x-request-id:
+      - 6fafe66e-16d3-40c4-be92-91e2f3601a54
+    status:
+      code: 404
+      message: Not Found
+version: 1
diff --git a/fsspec/implementations/tests/test_dbfs.py b/fsspec/implementations/tests/test_dbfs.py
index 66475b2..bd61205 100644
--- a/fsspec/implementations/tests/test_dbfs.py
+++ b/fsspec/implementations/tests/test_dbfs.py
@@ -129,11 +129,21 @@ def make_mock_diabetes_ds():
 
 
 @pytest.mark.vcr()
-def test_dbfs_file_listing(dbfsFS):
-    assert "/FileStore" in dbfsFS.ls("/", detail=False)
-    assert {"name": "/FileStore", "size": 0, "type": "directory"} in dbfsFS.ls(
-        "/", detail=True
+def test_dbfs_listing(dbfsFS):
+    dbfsFS.mkdir(
+        "/FileStore/fsspec_dbfs/project_root/empty_dir",
+        create_parents=True,
+        exist_ok=True,
     )
+    assert {
+        "name": "/FileStore/fsspec_dbfs/project_root",
+        "size": 0,
+        "type": "directory",
+    } in dbfsFS.ls("/FileStore/fsspec_dbfs", detail=True)
+    assert dbfsFS.ls("/FileStore/fsspec_dbfs/project_root/empty_dir", detail=True) == []
+    # Test that FileNotFoundError is raised when listing a missing directory
+    with pytest.raises(FileNotFoundError):
+        dbfsFS.ls("/FileStore/fsspec_dbfs/project_root/missing_directory", detail=True)
 
 
 @pytest.mark.vcr()
diff --git a/fsspec/implementations/tests/test_github.py b/fsspec/implementations/tests/test_github.py
index 32df94f..9f73348 100644
--- a/fsspec/implementations/tests/test_github.py
+++ b/fsspec/implementations/tests/test_github.py
@@ -1,3 +1,5 @@
+import pytest
+
 import fsspec
 
 
@@ -46,3 +48,17 @@ def test_github_ls():
     expected = {"brain_networks.csv", "mpg.csv", "penguins.csv", "README.md", "raw"}
     # check if the result is a subset of the expected files
     assert expected.issubset(ls_result)
+
+
+def test_github_rm():
+    # trying to remove a file without passing authentication should raise ValueError
+    fs = fsspec.filesystem("github", org="mwaskom", repo="seaborn-data")
+    with pytest.raises(ValueError):
+        fs.rm("mpg.csv")
+
+    # trying to remove a file which doesn't exist should raise FineNotFoundError
+    fs = fsspec.filesystem(
+        "github", org="mwaskom", repo="seaborn-data", username="user", token="token"
+    )
+    with pytest.raises(FileNotFoundError):
+        fs.rm("/this-file-doesnt-exist")