Repository: frictionlessdata/frictionless-py

Issue:
Zipping a package fails if there a sql/ckan/etc resources
# Overview

We need to fix how loader/parser/resource.memory/remote/multipart work. Inicate bigquery/ckan/sql by resource.service?


Discussion:

Patch:
diff --git a/frictionless/error.py b/frictionless/error.py
index 5a196f2e..b0a4a104 100644
--- a/frictionless/error.py
+++ b/frictionless/error.py
@@ -6,6 +6,7 @@ from .metadata import Metadata
 # effective as it can be very memory consumig. As an option we can store
 # raw data without rendering an error template to an error messsage.
 # Also, validation is disabled for performance reasons at the moment.
+# Allow creating from a descriptor (note needs to be optional)
 
 
 class Error(Metadata):
diff --git a/frictionless/file.py b/frictionless/file.py
index d9591e2b..190d3797 100644
--- a/frictionless/file.py
+++ b/frictionless/file.py
@@ -133,7 +133,7 @@ class File:
                         part = os.path.join(self.__basepath, part)
                     fullpath.append(part)
             else:  # for string paths
-                if not helpers.is_remote_path(path):
+                if not helpers.is_remote_path(path) and "://" not in path:
                     fullpath = os.path.join(self.__basepath, path)
 
         # Detect name
diff --git a/frictionless/package.py b/frictionless/package.py
index a6f8c66f..b7cb73df 100644
--- a/frictionless/package.py
+++ b/frictionless/package.py
@@ -1,3 +1,4 @@
+import os
 import json
 import zipfile
 import tempfile
@@ -561,74 +562,60 @@ class Package(Metadata):
         """
         return Package(descriptor=path, **options)
 
-    def to_zip(self, path, *, resolve=[], encoder_class=None):
+    def to_zip(self, path, *, encoder_class=None):
         """Save package to a zip
 
         Parameters:
             path (str): target path
-            resolve (str[]): Data sources to resolve.
-                For "memory" data it means saving them as CSV and including into ZIP.
-                For "remote" data it means downloading them and including into ZIP.
-                For example, `resolve=["memory", "remote"]`
             encoder_class (object): json encoder class
 
         Raises:
             FrictionlessException: on any error
         """
         try:
-            with zipfile.ZipFile(path, "w") as zip:
+            with zipfile.ZipFile(path, "w") as archive:
                 package_descriptor = self.to_dict()
                 for index, resource in enumerate(self.resources):
                     descriptor = package_descriptor["resources"][index]
 
-                    # Multipart data
-                    if resource.multipart:
-                        note = "Zipping multipart resource is not yet supported"
-                        raise FrictionlessException(errors.ResourceError(note=note))
+                    # Remote data
+                    if resource.remote:
+                        pass
 
                     # Memory data
                     elif resource.memory:
-                        if "memory" in resolve:
+                        if not isinstance(resource.data, list):
                             path = f"{resource.name}.csv"
                             descriptor["path"] = path
                             del descriptor["data"]
                             with tempfile.NamedTemporaryFile() as file:
                                 tgt = Resource(path=file.name, format="csv", trusted=True)
                                 resource.write(tgt)
-                                zip.write(file.name, path)
-                        elif not isinstance(resource.data, list):
-                            note = f"Use resolve argument to zip {resource.data}"
-                            raise FrictionlessException(errors.ResourceError(note=note))
+                                archive.write(file.name, path)
 
-                    # Remote data
-                    # NOTE:
-                    # sql/ckan/etc should go here (rename to network?)
-                    # we might need to add format.remote/network and update resource.remote
-                    elif resource.remote:
-                        if "remote" in resolve:
-                            path = f"{resource.name}.{resource.format}"
-                            descriptor["path"] = path
-                            with tempfile.NamedTemporaryFile() as file:
-                                # NOTE: we might rebase on using resource here
-                                with system.create_loader(resource) as loader:
-                                    while True:
-                                        chunk = loader.byte_stream.read(1024)
-                                        if not chunk:
-                                            break
-                                        file.write(chunk)
-                                    file.flush()
-                                zip.write(file.name, path)
+                    # Multipart data
+                    elif resource.multipart:
+                        for path, fullpath in zip(resource.path, resource.fullpath):
+                            if os.path.isfile(fullpath):
+                                if not helpers.is_safe_path(fullpath):
+                                    note = f'Zipping usafe "{fullpath}" is not supported'
+                                    error = errors.PackageError(note=note)
+                                    raise FrictionlessException(error)
+                                archive.write(fullpath, path)
 
                     # Local Data
                     else:
                         path = resource.path
-                        if not helpers.is_safe_path(path):
-                            path = f"{resource.name}.{resource.format}"
-                            descriptor["path"] = path
-                        zip.write(resource.fullpath, path)
+                        fullpath = resource.fullpath
+                        if os.path.isfile(fullpath):
+                            if not helpers.is_safe_path(fullpath):
+                                note = f'Zipping usafe "{fullpath}" is not supported'
+                                error = errors.PackageError(note=note)
+                                raise FrictionlessException(error)
+                            archive.write(fullpath, path)
 
                 # Metadata
-                zip.writestr(
+                archive.writestr(
                     "datapackage.json",
                     json.dumps(
                         package_descriptor,
diff --git a/frictionless/system.py b/frictionless/system.py
index fa4bc2a6..b7bb5f2c 100644
--- a/frictionless/system.py
+++ b/frictionless/system.py
@@ -48,6 +48,7 @@ class System:
         "create_check",
         "create_control",
         "create_dialect",
+        "create_error",
         "create_file",
         "create_loader",
         "create_parser",
@@ -111,6 +112,26 @@ class System:
                 return dialect
         return Dialect(descriptor)
 
+    def create_error(self, descriptor):
+        """Create errors
+
+        Parameters:
+            descriptor (dict): error descriptor
+
+        Returns:
+            Error: error
+        """
+        code = descriptor.get("code", "")
+        for func in self.methods["create_error"].values():
+            error = func(descriptor)
+            if error is not None:
+                return error
+        for Class in vars(import_module("frictionless.errors")).values():
+            if getattr(Class, "code", None) == code:
+                return Class(descriptor)
+        note = f'cannot create error "{code}". Try installing "frictionless-{code}"'
+        raise FrictionlessException(errors.Error(note=note))
+
     def create_file(self, source, **options):
         """Create file
 


Test patch:
diff --git a/tests/test_package.py b/tests/test_package.py
index af4ee4cf..68d1a94d 100644
--- a/tests/test_package.py
+++ b/tests/test_package.py
@@ -6,6 +6,7 @@ import zipfile
 from pathlib import Path
 from frictionless import Package, Resource, Layout, describe_package, helpers
 from frictionless import FrictionlessException
+from frictionless.plugins.sql import SqlDialect
 
 
 # General
@@ -767,18 +768,14 @@ def test_package_to_yaml(tmpdir):
 @pytest.mark.skipif(helpers.is_platform("macos"), reason="It doesn't work for Macos")
 @pytest.mark.skipif(helpers.is_platform("windows"), reason="It doesn't work for Windows")
 def test_package_to_zip(tmpdir):
-
-    # Write
-    target = os.path.join(tmpdir, "package.zip")
-    package = Package("data/package.json")
-    package.to_zip(target)
-
-    # Read
-    package = Package.from_zip(target)
-    assert package.name == "name"
-    assert package.get_resource("name").name == "name"
-    assert package.get_resource("name").path == "table.csv"
-    assert package.get_resource("name").read_rows() == [
+    path = os.path.join(tmpdir, "package.zip")
+    source = Package("data/package.json")
+    source.to_zip(path)
+    target = Package.from_zip(path)
+    assert target.name == "name"
+    assert target.get_resource("name").name == "name"
+    assert target.get_resource("name").path == "table.csv"
+    assert target.get_resource("name").read_rows() == [
         {"id": 1, "name": "english"},
         {"id": 2, "name": "中国人"},
     ]
@@ -786,18 +783,13 @@ def test_package_to_zip(tmpdir):
 
 @pytest.mark.skipif(helpers.is_platform("macos"), reason="It doesn't work for Macos")
 @pytest.mark.skipif(helpers.is_platform("windows"), reason="It doesn't work for Windows")
-def test_package_to_zip_withdir_path(tmpdir):
-
-    # Write
-    target = os.path.join(tmpdir, "package.zip")
-    resource = Resource(path="data/table.csv")
-    package = Package(resources=[resource])
-    package.to_zip(target)
-
-    # Read
-    package = Package.from_zip(target)
-    assert package.get_resource("table").path == "data/table.csv"
-    assert package.get_resource("table").read_rows() == [
+def test_package_to_zip_resource_path(tmpdir):
+    path = os.path.join(tmpdir, "package.zip")
+    source = Package(resources=[Resource(path="data/table.csv")])
+    source.to_zip(path)
+    target = Package.from_zip(path)
+    assert target.get_resource("table").path == "data/table.csv"
+    assert target.get_resource("table").read_rows() == [
         {"id": 1, "name": "english"},
         {"id": 2, "name": "中国人"},
     ]
@@ -805,18 +797,13 @@ def test_package_to_zip_withdir_path(tmpdir):
 
 @pytest.mark.skipif(helpers.is_platform("macos"), reason="It doesn't work for Macos")
 @pytest.mark.skipif(helpers.is_platform("windows"), reason="It doesn't work for Windows")
-def test_package_to_zip_absolute_path(tmpdir):
-
-    # Write
-    target = os.path.join(tmpdir, "package.zip")
-    resource = Resource(path=os.path.abspath("data/table.csv"))
-    package = Package(resources=[resource])
-    package.to_zip(target)
-
-    # Read
-    package = Package.from_zip(target)
-    assert package.get_resource("table").path == "table.csv"
-    assert package.get_resource("table").read_rows() == [
+def test_package_to_zip_resource_remote_path(tmpdir):
+    path = os.path.join(tmpdir, "package.zip")
+    source = Package(resources=[Resource(path=BASEURL % "data/table.csv")])
+    source.to_zip(path)
+    target = Package.from_zip(path)
+    assert target.get_resource("table").path == BASEURL % "data/table.csv"
+    assert target.get_resource("table").read_rows() == [
         {"id": 1, "name": "english"},
         {"id": 2, "name": "中国人"},
     ]
@@ -824,129 +811,58 @@ def test_package_to_zip_absolute_path(tmpdir):
 
 @pytest.mark.skipif(helpers.is_platform("macos"), reason="It doesn't work for Macos")
 @pytest.mark.skipif(helpers.is_platform("windows"), reason="It doesn't work for Windows")
-def test_package_to_zip_resolve_memory(tmpdir):
-
-    # Write
-    target = os.path.join(tmpdir, "package.zip")
-    resource = Resource(name="table", data=[["id", "name"], [1, "english"], [2, "中国人"]])
-    package = Package(resources=[resource])
-    package.to_zip(target, resolve=["memory"])
-
-    # Read
-    package = Package.from_zip(target)
-    assert package.get_resource("table").path == "table.csv"
-    assert package.get_resource("table").read_rows() == [
+def test_package_to_zip_resource_memory_inline(tmpdir):
+    path = os.path.join(tmpdir, "package.zip")
+    data = [["id", "name"], [1, "english"], [2, "中国人"]]
+    source = Package(resources=[Resource(name="table", data=data)])
+    source.to_zip(path)
+    target = Package.from_zip(path)
+    assert target.get_resource("table").data == data
+    assert target.get_resource("table").read_rows() == [
         {"id": 1, "name": "english"},
         {"id": 2, "name": "中国人"},
     ]
 
 
-@pytest.mark.xfail(reason="zipping services")
 @pytest.mark.skipif(helpers.is_platform("macos"), reason="It doesn't work for Macos")
 @pytest.mark.skipif(helpers.is_platform("windows"), reason="It doesn't work for Windows")
-def test_package_to_zip_resolve_memory_sql(tmpdir, database_url):
-
-    # Write
-    target = os.path.join(tmpdir, "package.zip")
-    resource = Resource(database_url, dialect={"table": "table"})
-    package = Package(resources=[resource])
-    package.to_zip(target, resolve=["memory"])
-
-    # Read
-    package = Package.from_zip(target)
-    assert package.get_resource("table").path == "table.csv"
-    assert package.get_resource("table").read_rows() == [
+def test_package_to_zip_resource_memory_function(tmpdir):
+    path = os.path.join(tmpdir, "package.zip")
+    data = lambda: [["id", "name"], [1, "english"], [2, "中国人"]]
+    source = Package(resources=[Resource(name="table", data=data)])
+    source.to_zip(path)
+    target = Package.from_zip(path)
+    assert target.get_resource("table").path == "table.csv"
+    assert target.get_resource("table").read_rows() == [
         {"id": 1, "name": "english"},
         {"id": 2, "name": "中国人"},
     ]
 
 
-@pytest.mark.vcr
 @pytest.mark.skipif(helpers.is_platform("macos"), reason="It doesn't work for Macos")
 @pytest.mark.skipif(helpers.is_platform("windows"), reason="It doesn't work for Windows")
-def test_package_to_zip_resolve_remote(tmpdir):
-
-    # Write
-    target = os.path.join(tmpdir, "package.zip")
-    resource = Resource(path=BASEURL % "data/table.csv")
-    package = Package(resources=[resource])
-    package.to_zip(target, resolve=["remote"])
-
-    # Read
-    package = Package.from_zip(target)
-    assert package.get_resource("table").path == "table.csv"
-    assert package.get_resource("table").read_rows() == [
+def test_package_to_zip_resource_sql(tmpdir, database_url):
+    path = os.path.join(tmpdir, "package.zip")
+    dialect = SqlDialect(table="table")
+    source = Package(resources=[Resource(database_url, name="table", dialect=dialect)])
+    source.to_zip(path)
+    target = Package.from_zip(path)
+    assert target.get_resource("table").path == database_url
+    assert target.get_resource("table").read_rows() == [
         {"id": 1, "name": "english"},
         {"id": 2, "name": "中国人"},
     ]
 
 
-@pytest.mark.vcr
 @pytest.mark.skipif(helpers.is_platform("macos"), reason="It doesn't work for Macos")
 @pytest.mark.skipif(helpers.is_platform("windows"), reason="It doesn't work for Windows")
-def test_package_to_zip_resolve_memory_and_remote(tmpdir):
-
-    # Write
-    target = os.path.join(tmpdir, "package.zip")
-    resource1 = Resource(name="name1", data=[["id", "name"], [1, "english"], [2, "中国人"]])
-    resource2 = Resource(name="name2", path=BASEURL % "data/table.csv")
-    package = Package(resources=[resource1, resource2])
-    package.to_zip(target, resolve=["memory", "remote"])
-
-    # Read
-    package = Package.from_zip(target)
-    assert package.get_resource("name1").path == "name1.csv"
-    assert package.get_resource("name1").read_rows() == [
-        {"id": 1, "name": "english"},
-        {"id": 2, "name": "中国人"},
-    ]
-    assert package.get_resource("name2").path == "name2.csv"
-    assert package.get_resource("name2").read_rows() == [
-        {"id": 1, "name": "english"},
-        {"id": 2, "name": "中国人"},
-    ]
-
-
-@pytest.mark.vcr
-@pytest.mark.skipif(helpers.is_platform("macos"), reason="It doesn't work for Macos")
-@pytest.mark.skipif(helpers.is_platform("windows"), reason="It doesn't work for Windows")
-def test_package_to_zip_source_remote(tmpdir):
-
-    # Write
-    path = BASEURL % "data/table.csv"
-    target = os.path.join(tmpdir, "package.zip")
-    package = Package(name="name", resources=[{"name": "name", "path": path}])
-    package.to_zip(target)
-
-    # Read
-    package = Package.from_zip(target)
-    assert package == {
-        "name": "name",
-        "resources": [{"name": "name", "path": path}],
-    }
-    assert package.get_resource("name").read_rows() == [
-        {"id": 1, "name": "english"},
-        {"id": 2, "name": "中国人"},
-    ]
-
-
-@pytest.mark.skipif(helpers.is_platform("macos"), reason="It doesn't work for Macos")
-@pytest.mark.skipif(helpers.is_platform("windows"), reason="It doesn't work for Windows")
-def test_package_to_zip_source_inline(tmpdir):
-
-    # Read
-    target = os.path.join(tmpdir, "package.zip")
-    data = [["id", "name"], ["1", "english"], ["2", "中国人"]]
-    package = Package(name="name", resources=[{"name": "name", "data": data}])
-    package.to_zip(target)
-
-    # Write
-    package = Package.from_zip(target)
-    assert package == {
-        "name": "name",
-        "resources": [{"name": "name", "data": data}],
-    }
-    assert package.get_resource("name").read_rows() == [
+def test_package_to_zip_resource_multipart(tmpdir, database_url):
+    path = os.path.join(tmpdir, "package.zip")
+    source = Package(resources=[Resource(path=["data/chunk1.csv", "data/chunk2.csv"])])
+    source.to_zip(path)
+    target = Package.from_zip(path)
+    assert target.get_resource("chunk").path == ["data/chunk1.csv", "data/chunk2.csv"]
+    assert target.get_resource("chunk").read_rows() == [
         {"id": 1, "name": "english"},
         {"id": 2, "name": "中国人"},
     ]