pull _is_file checks to get_listing

The parsing utility should not hit remote servers at all.
Also returning `None` does not make sense for `parse_listing_uri`
when the uri is a file since there is no listing.

It's better to keep this function naive than to add other logic into
it. And I think `get_listing` is a better place for that, to have all
kinds of logic with side-effects.

The only other place where this function is used besides `get_listing` is in `DatasetDependency.dataset_name`.
In that case, a dataset already exists and since there can be no listing
for a file uri, it's safe to assume that the uri is either a glob or a
directory path.

https://github.com/iterative/datachain/blob/e45a62f43cad47a162345f01e3a2f9a1fff8f47a/src/datachain/dataset.py#L94

I stumbled upon this when I was investigating why a unit-test was slow.
Turns out it was hitting the remote with a fake s3 path for `is_file()`,
which was failing and taking 4-5 seconds to run.

https://github.com/iterative/datachain/blob/e45a62f43cad47a162345f01e3a2f9a1fff8f47a/tests/unit/test_dataset.py#L88-L99

Author: skshetry

src/datachain/dataset.py

@@ -91,7 +91,7 @@ def dataset_name(self) -> str:
         if self.type == DatasetDependencyType.DATASET:
             return self.name
 
-        list_dataset_name, _, _ = parse_listing_uri(self.name.strip("/"), None, {})
+        list_dataset_name, _, _ = parse_listing_uri(self.name.strip("/"), {})
         assert list_dataset_name
         return list_dataset_name
 

src/datachain/lib/listing.py

@@ -103,17 +103,12 @@ def _isfile(client: "Client", path: str) -> bool:
         return False
 
 
-def parse_listing_uri(uri: str, cache, client_config) -> tuple[Optional[str], str, str]:
+def parse_listing_uri(uri: str, client_config) -> tuple[str, str, str]:
     """
     Parsing uri and returns listing dataset name, listing uri and listing path
     """
     client_config = client_config or {}
-    client = Client.get_client(uri, cache, **client_config)
     storage_uri, path = Client.parse_url(uri)
-    telemetry.log_param("client", client.PREFIX)
-
-    if not uri.endswith("/") and _isfile(client, uri):
-        return None, f"{storage_uri}/{path.lstrip('/')}", path
     if uses_glob(path):
         lst_uri_path = posixpath.dirname(path)
     else:
@@ -157,13 +152,15 @@ def get_listing(
     client_config = catalog.client_config
 
     client = Client.get_client(uri, cache, **client_config)
-    ds_name, list_uri, list_path = parse_listing_uri(uri, cache, client_config)
-    listing = None
+    telemetry.log_param("client", client.PREFIX)
 
-    # if we don't want to use cached dataset (e.g. for a single file listing)
-    if not ds_name:
-        return None, list_uri, list_path, False
+    # we don't want to use cached dataset (e.g. for a single file listing)
+    if not uri.endswith("/") and _isfile(client, uri):
+        storage_uri, path = Client.parse_url(uri)
+        return None, f"{storage_uri}/{path.lstrip('/')}", path, False
 
+    ds_name, list_uri, list_path = parse_listing_uri(uri, client_config)
+    listing = None
     listings = [
         ls for ls in catalog.listings() if not ls.is_expired and ls.contains(ds_name)
     ]

tests/func/test_catalog.py

@@ -17,9 +17,7 @@
 
 
 def listing_stats(uri, catalog):
-    list_dataset_name, _, _ = parse_listing_uri(
-        uri, catalog.cache, catalog.client_config
-    )
+    list_dataset_name, _, _ = parse_listing_uri(uri, catalog.client_config)
     dataset = catalog.get_dataset(list_dataset_name)
     return catalog.dataset_stats(dataset.name, dataset.latest_version)
 

tests/func/test_datachain.py

@@ -110,7 +110,7 @@ def test_from_storage_reindex_expired(tmp_dir, test_session):
     os.mkdir(tmp_dir)
     uri = tmp_dir.as_uri()
 
-    lst_ds_name = parse_listing_uri(uri, catalog.cache, catalog.client_config)[0]
+    lst_ds_name = parse_listing_uri(uri, catalog.client_config)[0]
 
     pd.DataFrame({"name": ["Alice", "Bob"]}).to_parquet(tmp_dir / "test1.parquet")
     assert DataChain.from_storage(uri, session=test_session).count() == 1
@@ -138,7 +138,7 @@ def test_from_storage_partials(cloud_test_catalog):
     catalog = session.catalog
 
     def _list_dataset_name(uri: str) -> str:
-        name = parse_listing_uri(uri, catalog.cache, catalog.client_config)[0]
+        name = parse_listing_uri(uri, catalog.client_config)[0]
         assert name
         return name
 
@@ -182,7 +182,7 @@ def test_from_storage_partials_with_update(cloud_test_catalog):
     catalog = session.catalog
 
     def _list_dataset_name(uri: str) -> str:
-        name = parse_listing_uri(uri, catalog.cache, catalog.client_config)[0]
+        name = parse_listing_uri(uri, catalog.client_config)[0]
         assert name
         return name
 

tests/func/test_datasets.py

@@ -862,7 +862,7 @@ def test_dataset_storage_dependencies(cloud_test_catalog, cloud_type, indirect):
     ds_name = "some_ds"
     DataChain.from_storage(uri, session=session).save(ds_name)
 
-    lst_ds_name, _, _ = parse_listing_uri(uri, catalog.cache, catalog.client_config)
+    lst_ds_name, _, _ = parse_listing_uri(uri, catalog.client_config)
     lst_dataset = catalog.metastore.get_dataset(lst_ds_name)
 
     assert [

tests/func/test_listing.py

@@ -38,7 +38,7 @@ def test_parse_listing_uri(cloud_test_catalog, cloud_type):
     ctc = cloud_test_catalog
     catalog = ctc.catalog
     dataset_name, listing_uri, listing_path = parse_listing_uri(
-        f"{ctc.src_uri}/dogs", catalog.cache, catalog.client_config
+        f"{ctc.src_uri}/dogs", catalog.client_config
     )
     assert dataset_name == f"lst__{ctc.src_uri}/dogs/"
     assert listing_uri == f"{ctc.src_uri}/dogs/"
@@ -57,7 +57,7 @@ def test_parse_listing_uri_with_glob(cloud_test_catalog):
     ctc = cloud_test_catalog
     catalog = ctc.catalog
     dataset_name, listing_uri, listing_path = parse_listing_uri(
-        f"{ctc.src_uri}/dogs/*", catalog.cache, catalog.client_config
+        f"{ctc.src_uri}/dogs/*", catalog.client_config
     )
     assert dataset_name == f"lst__{ctc.src_uri}/dogs/"
     assert listing_uri == f"{ctc.src_uri}/dogs"