Merge pull request #943 from koordinates/rename-byod

Rename byod to "linked-storage"

kart.spec

@@ -156,7 +156,7 @@ pyi_analysis = Analysis(
         # TODO: improve this somehow
         *collect_submodules('kart'),
         *collect_submodules('kart.annotations'),
-        *collect_submodules('kart.byod'),
+        *collect_submodules('kart.linked_storage'),
         *collect_submodules('kart.lfs_commands'),
         *collect_submodules('kart.point_cloud'),
         *collect_submodules('kart.sqlalchemy'),

kart/cli.py

@@ -54,8 +54,8 @@
     "point_cloud.import_": {"point-cloud-import"},
     "install": {"install"},
     "add_dataset": {"add-dataset"},
-    "byod.point_cloud_import": {"byod-point-cloud-import"},
-    "byod.raster_import": {"byod-raster-import"},
+    "linked_storage.point_cloud_import": {"linked-point-cloud-import"},
+    "linked_storage.raster_import": {"linked-raster-import"},
 }
 
 # These commands aren't valid Python symbols, even when we change dash to underscore.

kart/cli_util.py

@@ -170,7 +170,6 @@ def __init__(self, *args, **kwargs):
     def handle_parse_result(self, ctx, opts, args):
         current_opt: bool = self.name in opts
         for other_name in self.exclusive_with:
-
             if other_name in opts:
                 if current_opt:
                     other = [x for x in ctx.command.params if x.name == other_name][0]
@@ -412,3 +411,16 @@ def find_param(ctx_or_params, name):
         if param.name == name:
             return param
     raise RuntimeError(f"Couldn't find param: {name}")
+
+
+def forward_context_to_command(ctx, command):
+    """
+    Given the current context and a new command (not the current command), run the new command but
+    reusing the same overall context including command line arguments.
+    The new command should accept arguments in much the same form as the current command.
+    If any arguments cannot be parsed by the new command, it will fail with a UsageError as per usual.
+    This could be acceptable so long as it is clear to the user what is unsupported and why.
+    """
+    subctx = command.make_context(command.name, ctx.unparsed_args)
+    subctx.obj = ctx.obj
+    subctx.forward(command)

kart/import_.py

@@ -5,6 +5,7 @@
     call_and_exit_flag,
     KartCommand,
     find_param,
+    forward_context_to_command,
 )
 from kart.completion_shared import file_path_completer
 from kart.import_sources import from_spec, suggest_specs
@@ -60,6 +61,16 @@ def list_import_formats(ctx):
 @click.option(
     "--dataset-path", "--dataset", "ds_path", help="The dataset's path once imported"
 )
+@click.option(
+    "--link",
+    "do_link",
+    is_flag=True,
+    help=(
+        "Link the created dataset to the original source location, so that the original source location is treated as "
+        "the authoritative source for the given data and data is fetched from there if needed. Only supported for "
+        "tile-based datasets."
+    ),
+)
 @click.argument(
     "args",
     nargs=-1,
@@ -134,8 +145,13 @@ def import_(ctx, args, **kwargs):
             f"Try one of the following:\n{suggest_specs()}"
         )
 
-    import_cmd = import_source_type.import_cmd
+    if kwargs.get("do_link"):
+        import_cmd = import_source_type.linked_import_cmd
+        if import_cmd is None:
+            raise click.UsageError(
+                "--link is not supported for vector or tabular imports"
+            )
+    else:
+        import_cmd = import_source_type.import_cmd
 
-    subctx = import_cmd.make_context(import_cmd.name, ctx.unparsed_args)
-    subctx.obj = ctx.obj
-    subctx.forward(import_cmd)
+    forward_context_to_command(ctx, import_cmd)

kart/import_sources.py

@@ -33,6 +33,17 @@ def import_cmd(self):
 
             return raster_import
 
+    @property
+    def linked_import_cmd(self):
+        if self is self.POINT_CLOUD:
+            from kart.linked_storage.point_cloud_import import linked_point_cloud_import
+
+            return linked_point_cloud_import
+        elif self is self.RASTER:
+            from kart.linked_storage.raster_import import linked_raster_import
+
+            return linked_raster_import
+
     @property
     def import_source_class(self):
         if self is self.SQLALCHEMY_TABLE:
@@ -80,6 +91,10 @@ def __init__(
     def import_cmd(self):
         return self.import_type.import_cmd
 
+    @property
+    def linked_import_cmd(self):
+        return self.import_type.linked_import_cmd
+
     @property
     def import_source_class(self):
         return self.import_type.import_source_class

kart/lfs_commands/__init__.py

@@ -376,7 +376,7 @@ def _do_fetch_from_urls(repo, urls_and_lfs_oids, quiet=False):
     )
     if non_s3_url:
         raise NotImplementedError(
-            f"Invalid URL - only S3 URLs are currently supported for BYOD repos: {non_s3_url}"
+            f"Invalid URL - only S3 URLs are currently supported for linked-storage datasets: {non_s3_url}"
         )
 
     urls_and_paths_and_oids = [

kart/byod/importer.py → kart/linked_storage/importer.py

@@ -11,7 +11,7 @@
 L = logging.getLogger(__name__)
 
 
-class ByodTileImporter:
+class LinkedTileImporter:
     """
     Subclassable logic for importing the metadata from tile-based datasets,
     while leaving the data in-place on existing hosted storage.

kart/byod/point_cloud_import.py → kart/linked_storage/point_cloud_import.py

@@ -2,18 +2,27 @@
 
 import click
 
-from kart.byod.importer import ByodTileImporter
+from kart.linked_storage.importer import LinkedTileImporter
 from kart.cli_util import StringFromFile, MutexOption, KartCommand
 from kart.point_cloud.import_ import PointCloudImporter
 from kart.point_cloud.metadata_util import extract_pc_tile_metadata
-from kart.s3_util import get_hash_and_size_of_s3_object, fetch_from_s3
+from kart.s3_util import get_hash_and_size_of_s3_object
 
 
 L = logging.getLogger(__name__)
 
 
-@click.command("byod-point-cloud-import", hidden=True, cls=KartCommand)
+@click.command("linked-point-cloud-import", hidden=True, cls=KartCommand)
 @click.pass_context
+@click.option(
+    "--convert-to-copc/--no-convert-to-copc",
+    "--cloud-optimized/--no-cloud-optimized",
+    "--cloud-optimised/--no-cloud-optimised",
+    " /--preserve-format",
+    is_flag=True,
+    default=None,
+    help="Whether to convert all non-COPC LAS or LAZ files to COPC LAZ files, or to import all files in their native format.",
+)
 @click.option(
     "--message",
     "-m",
@@ -82,13 +91,26 @@
     hidden=True,
 )
 @click.option("--dataset-path", "--dataset", help="The dataset's path once imported")
+@click.option(
+    "--link",
+    "do_link",
+    is_flag=True,
+    default=None,
+    hidden=True,
+    help=(
+        "Link the created dataset to the original source location, so that the original source location is treated as "
+        "the authoritative source for the given data and data is fetched from there if needed. Only supported for "
+        "tile-based datasets."
+    ),
+)
 @click.argument(
     "sources",
     nargs=-1,
     metavar="SOURCE [SOURCES...]",
 )
-def byod_point_cloud_import(
+def linked_point_cloud_import(
     ctx,
+    convert_to_copc,
     message,
     do_checkout,
     replace_existing,
@@ -98,16 +120,27 @@ def byod_point_cloud_import(
     allow_empty,
     num_workers,
     dataset_path,
+    do_link,
     sources,
 ):
     """
-    Experimental. Import a dataset of point-cloud tiles from S3. Doesn't fetch the tiles, does store the tiles original location.
+    Import a dataset of point-cloud tiles from S3. Doesn't fetch the tiles, does store the tiles original location.
 
     SOURCES should be one or more LAZ or LAS files (or wildcards that match multiple LAZ or LAS files).
     """
+    if do_link is False:
+        # This is here for technical reasons - all the options are forwarded from one command to another, including --link.
+        # In practise we don't expect the user to set --link at all if they are also manually calling this (hidden) command.
+        raise click.UsageError("Can't do a linked-import with --link=false")
+    if convert_to_copc:
+        raise click.UsageError(
+            "Sorry, converting a linked dataset to COPC is not supported - "
+            "the data must remain in its original location and its original format as the authoritative source."
+        )
+
     repo = ctx.obj.repo
 
-    ByodPointCloudImporter(
+    LinkedPointCloudImporter(
         repo=repo,
         ctx=ctx,
         convert_to_cloud_optimized=False,
@@ -124,7 +157,7 @@ def byod_point_cloud_import(
     ).import_tiles()
 
 
-class ByodPointCloudImporter(ByodTileImporter, PointCloudImporter):
+class LinkedPointCloudImporter(LinkedTileImporter, PointCloudImporter):
     def extract_tile_metadata(self, tile_location):
         oid_and_size = get_hash_and_size_of_s3_object(tile_location)
         return None, extract_pc_tile_metadata(tile_location, oid_and_size=oid_and_size)

kart/byod/raster_import.py → kart/linked_storage/raster_import.py

@@ -2,18 +2,27 @@
 
 import click
 
-from kart.byod.importer import ByodTileImporter
+from kart.linked_storage.importer import LinkedTileImporter
 from kart.cli_util import StringFromFile, MutexOption, KartCommand
 from kart.raster.import_ import RasterImporter
 from kart.raster.metadata_util import extract_raster_tile_metadata
-from kart.s3_util import get_hash_and_size_of_s3_object, fetch_from_s3
+from kart.s3_util import get_hash_and_size_of_s3_object
 
 
 L = logging.getLogger(__name__)
 
 
-@click.command("byod-raster-import", hidden=True, cls=KartCommand)
+@click.command("linked-raster-import", hidden=True, cls=KartCommand)
 @click.pass_context
+@click.option(
+    "--convert-to-cog/--no-convert-to-cog",
+    "--cloud-optimized/--no-cloud-optimized",
+    "--cloud-optimised/--no-cloud-optimised",
+    " /--preserve-format",
+    is_flag=True,
+    default=None,
+    help="Whether to convert all GeoTIFFs to COGs (Cloud Optimized GeoTIFFs), or to import all files in their native format.",
+)
 @click.option(
     "--message",
     "-m",
@@ -82,13 +91,26 @@
     hidden=True,
 )
 @click.option("--dataset-path", "--dataset", help="The dataset's path once imported")
+@click.option(
+    "--link",
+    "do_link",
+    is_flag=True,
+    default=None,
+    hidden=True,
+    help=(
+        "Link the created dataset to the original source location, so that the original source location is treated as "
+        "the authoritative source for the given data and data is fetched from there if needed. Only supported for "
+        "tile-based datasets."
+    ),
+)
 @click.argument(
     "sources",
     nargs=-1,
     metavar="SOURCE [SOURCES...]",
 )
-def byod_raster_import(
+def linked_raster_import(
     ctx,
+    convert_to_cog,
     message,
     do_checkout,
     replace_existing,
@@ -98,16 +120,27 @@ def byod_raster_import(
     allow_empty,
     num_workers,
     dataset_path,
+    do_link,
     sources,
 ):
     """
-    Experimental. Import a dataset of raster tiles from S3. Doesn't fetch the tiles, does store the tiles original location.
+    Import a dataset of raster tiles from S3. Doesn't fetch the tiles, does store the tiles original location.
 
     SOURCES should be one or more GeoTIFF files (or wildcards that match multiple GeoTIFF files).
     """
+    if do_link is False:
+        # This is here for technical reasons - all the options are forwarded from one command to another, including --link.
+        # In practise we don't expect the user to set --link at all if they are also manually calling this (hidden) command.
+        raise click.UsageError("Can't do a linked import with --link=false")
+    if convert_to_cog:
+        raise click.UsageError(
+            "Sorry, converting a linked dataset to COG is not supported - "
+            "the data must remain in its original location and its original format as the authoritative source."
+        )
+
     repo = ctx.obj.repo
 
-    ByodRasterImporter(
+    LinkedRasterImporter(
         repo=repo,
         ctx=ctx,
         convert_to_cloud_optimized=False,
@@ -124,7 +157,7 @@ def byod_raster_import(
     ).import_tiles()
 
 
-class ByodRasterImporter(ByodTileImporter, RasterImporter):
+class LinkedRasterImporter(LinkedTileImporter, RasterImporter):
     def extract_tile_metadata(self, tile_location):
         oid_and_size = get_hash_and_size_of_s3_object(tile_location)
         return None, extract_raster_tile_metadata(

kart/point_cloud/import_.py

@@ -2,7 +2,12 @@
 
 import click
 
-from kart.cli_util import StringFromFile, MutexOption, KartCommand
+from kart.cli_util import (
+    StringFromFile,
+    MutexOption,
+    KartCommand,
+    forward_context_to_command,
+)
 from kart.completion_shared import file_path_completer
 from kart.exceptions import InvalidOperation, INVALID_FILE_FORMAT
 from kart.parse_args import parse_import_sources_and_datasets
@@ -98,6 +103,15 @@
     hidden=True,
 )
 @click.option("--dataset-path", "--dataset", help="The dataset's path once imported")
+@click.option(
+    "--link",
+    "do_link",
+    is_flag=True,
+    help=(
+        "Link the created dataset to the original source location, so that the original source location is treated as "
+        "the authoritative source for the given data and data is fetched from there if needed."
+    ),
+)
 @click.argument(
     "args",
     nargs=-1,
@@ -116,13 +130,20 @@ def point_cloud_import(
     allow_empty,
     num_workers,
     dataset_path,
+    do_link,
     args,
 ):
     """
     Import a dataset of point-cloud tiles.
 
     SOURCES should be one or more LAZ or LAS files (or wildcards that match multiple LAZ or LAS files).
     """
+    if do_link:
+        from kart.linked_storage.point_cloud_import import linked_point_cloud_import
+
+        forward_context_to_command(ctx, linked_point_cloud_import)
+        return
+
     repo = ctx.obj.repo
 
     sources, datasets = parse_import_sources_and_datasets(args)