Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename byod #943

Merged
merged 3 commits into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion kart.spec
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down
4 changes: 2 additions & 2 deletions kart/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
14 changes: 13 additions & 1 deletion kart/cli_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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)
24 changes: 20 additions & 4 deletions kart/import_.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
15 changes: 15 additions & 0 deletions kart/import_sources.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion kart/lfs_commands/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down
File renamed without changes.
2 changes: 1 addition & 1 deletion kart/byod/importer.py → kart/linked_storage/importer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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(
Expand Down
23 changes: 22 additions & 1 deletion kart/point_cloud/import_.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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)
Expand Down
Loading
Loading