Skip to content

Item type and asset type validation in Data and Subscriptions #905

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

Merged
merged 36 commits into from
Apr 7, 2023
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
47c5002
Added validation for item types and thier options in an click epilog.
kevinlacaille Mar 31, 2023
b861774
Added validation for item types in get_asset() in the data client.
kevinlacaille Mar 31, 2023
5deba79
Added validation for item types in request.
kevinlacaille Mar 31, 2023
9358029
Added function to get supported assets and a validation function.
kevinlacaille Mar 31, 2023
19f088b
Added validation for asset types.
kevinlacaille Mar 31, 2023
688b23b
Added @translate_exceptions, but doesnt seem to work... hmmm
kevinlacaille Mar 31, 2023
19d5ac1
Added validation in try/except, but not sure that is the best way. TBD
kevinlacaille Mar 31, 2023
d3e5fe5
Removed annoyinng index into asset_types and now can validate all ass…
kevinlacaille Mar 31, 2023
a1819c9
Addeds tests for validate_asset_type.
kevinlacaille Mar 31, 2023
bd43ec2
Added tests for get_supported_assets.
kevinlacaille Mar 31, 2023
383deba
Bug fix: Flipped the order of item_type and name.
kevinlacaille Apr 3, 2023
6c7faa1
Bug fix: item_types doesnt need a list
kevinlacaille Apr 3, 2023
44a82b4
Removed list around item_types because item_types are now a click.Cho…
kevinlacaille Apr 3, 2023
4328639
Added translation of Spcification exception.
kevinlacaille Apr 3, 2023
adb4bba
Unsupported asset type.
kevinlacaille Apr 3, 2023
7df6274
Undoing bug fix. Will fix in a future ticket, #908.
kevinlacaille Apr 4, 2023
7e8fdaa
Changed item types back to a comma separated string.
kevinlacaille Apr 6, 2023
7e98d59
Validate asset types for each item type.
kevinlacaille Apr 6, 2023
51c4485
Added test for multiple item types and asset types.
kevinlacaille Apr 6, 2023
6a8eb49
Merge branch 'main' into item-type-case-insensitive-orders-subs-903
kevinlacaille Apr 6, 2023
bb70f60
Merge branch 'main' into item-type-case-insensitive-orders-subs-903
kevinlacaille Apr 6, 2023
bf0d3f5
Readded list to item_types.
kevinlacaille Apr 6, 2023
4ccb908
Unparamaterized test_request_catalog_success.
kevinlacaille Apr 6, 2023
baae55a
Added docstring and postfix to test_validate_asset_type.
kevinlacaille Apr 6, 2023
608fc51
Added callback function for item type.
kevinlacaille Apr 6, 2023
abbd994
Added docstring to validate_asset_type.
kevinlacaille Apr 6, 2023
b39f611
Reformatted data tests and duplicated data tests for subscriptions te…
kevinlacaille Apr 6, 2023
3512f80
Reformatted asset_type validation. Now accepts multiple asset types f…
kevinlacaille Apr 6, 2023
bd6b7df
ClientError raised if more than 1 item type is provided.
kevinlacaille Apr 6, 2023
f1af245
linting
kevinlacaille Apr 6, 2023
0ad9816
linting
kevinlacaille Apr 6, 2023
a3900d2
Renamed filename to better represent whats being tested.
kevinlacaille Apr 7, 2023
5e41ec0
Replace awkward text wrapping with nice continuous text.
kevinlacaille Apr 7, 2023
2d62bd2
Removed item type parametrization.
kevinlacaille Apr 7, 2023
c0cb296
Only test against one supported asset type.
kevinlacaille Apr 7, 2023
03d3308
Renamed filename to better represent whats being tested.
kevinlacaille Apr 7, 2023
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
12 changes: 4 additions & 8 deletions planet/cli/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -407,10 +407,6 @@ async def search_run(ctx, search_id, sort, limit, pretty):
echo_json(item, pretty)


# TODO: search-update
# TODO: search-delete


@data.command(epilog=valid_item_string)
@click.pass_context
@translate_exceptions
Expand Down Expand Up @@ -471,7 +467,7 @@ async def search_delete(ctx, search_id):
await cl.delete_search(search_id)


@data.command()
@data.command(epilog=valid_item_string)
@click.pass_context
@translate_exceptions
@coro
Expand Down Expand Up @@ -514,7 +510,7 @@ async def search_update(ctx,
echo_json(items, pretty)


@data.command()
@data.command(epilog=valid_item_string)
@click.pass_context
@translate_exceptions
@coro
Expand Down Expand Up @@ -576,7 +572,7 @@ async def asset_download(ctx,
cl.validate_checksum(asset, path)


@data.command()
@data.command(epilog=valid_item_string)
@click.pass_context
@translate_exceptions
@coro
Expand All @@ -590,7 +586,7 @@ async def asset_activate(ctx, item_type, item_id, asset_type):
await cl.activate_asset(asset)


@data.command()
@data.command(epilog=valid_item_string)
@click.pass_context
@translate_exceptions
@coro
Expand Down
12 changes: 9 additions & 3 deletions planet/cli/subscriptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
from .session import CliSession
from planet.clients.subscriptions import SubscriptionsClient
from .. import subscription_request
from ..specs import get_item_types

ALL_ITEM_TYPES = get_item_types()
valid_item_string = "Valid entries for ITEM_TYPES: " + "|".join(ALL_ITEM_TYPES)


@asynccontextmanager
Expand Down Expand Up @@ -161,6 +165,7 @@ async def list_subscription_results_cmd(ctx,


@subscriptions.command()
@translate_exceptions
@click.option('--name',
required=True,
type=str,
Expand Down Expand Up @@ -192,11 +197,12 @@ def request(name, source, delivery, notifications, tools, pretty):
echo_json(res, pretty)


@subscriptions.command()
@subscriptions.command(epilog=valid_item_string)
@translate_exceptions
@click.option('--item-types',
required=True,
type=types.CommaSeparatedString(),
help='One or more comma-separated item types.')
help='Item type for requested item ids.',
type=types.CommaSeparatedString())
@click.option('--asset-types',
required=True,
type=types.CommaSeparatedString(),
Expand Down
1 change: 1 addition & 0 deletions planet/clients/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,7 @@ async def get_asset(self,
planet.exceptions.ClientError: If asset type identifier is not
valid.
"""
item_type_id = validate_item_type(item_type_id)
assets = await self.list_item_assets(item_type_id, item_id)

try:
Expand Down
21 changes: 21 additions & 0 deletions planet/specs.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,13 @@ def validate_supported_bundles(item_type, bundle, all_product_bundles):
return _validate_field(bundle, supported_bundles, 'bundle')


def validate_asset_type(item_type, asset_type):
item_type = validate_item_type(item_type)
supported_assets = get_supported_assets(item_type)

return _validate_field(asset_type, supported_assets, 'asset_type')


def _get_product_bundle_spec():
with open(DATA_DIR / PRODUCT_BUNDLE_SPEC_NAME) as f:
data = json.load(f)
Expand Down Expand Up @@ -162,3 +169,17 @@ def get_item_types(product_bundle=None):
for bundle in get_product_bundles()))

return item_types


def get_supported_assets(item_type):
'''Get all assets supported by a given item type.'''
item_type = validate_item_type(item_type)
supported_bundles = get_product_bundles(item_type)
spec = _get_product_bundle_spec()
supported_assets = [
spec['bundles'][bundle]["assets"][item_type]
for bundle in supported_bundles
]
supported_assets = list(set(list(itertools.chain(*supported_assets))))

return supported_assets
7 changes: 7 additions & 0 deletions planet/subscription_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,13 @@ def catalog_source(
planet.exceptions.ClientError: If start_time or end_time are not valid
datetimes
'''
for i in range(len(item_types)):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code errors out when I use multiple item-types:

planet-client-python$> planet subscriptions request-catalog --item-types REScene,PSScene --asset-types basic_analytic_b2 --geometry geometry.geojson --start-time 2022-02-02
Traceback (most recent call last):
  File "/Users/jennifer.kyle/.pyenv/versions/planet-client-python-3.7.9/bin/planet", line 11, in <module>
    load_entry_point('planet', 'console_scripts', 'planet')()
  File "/Users/jennifer.kyle/.pyenv/versions/planet-client-python-3.7.9/lib/python3.7/site-packages/click/core.py", line 1134, in __call__
    return self.main(*args, **kwargs)
  File "/Users/jennifer.kyle/.pyenv/versions/planet-client-python-3.7.9/lib/python3.7/site-packages/click/core.py", line 1059, in main
    rv = self.invoke(ctx)
  File "/Users/jennifer.kyle/.pyenv/versions/planet-client-python-3.7.9/lib/python3.7/site-packages/click/core.py", line 1665, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/Users/jennifer.kyle/.pyenv/versions/planet-client-python-3.7.9/lib/python3.7/site-packages/click/core.py", line 1665, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/Users/jennifer.kyle/.pyenv/versions/planet-client-python-3.7.9/lib/python3.7/site-packages/click/core.py", line 1401, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/Users/jennifer.kyle/.pyenv/versions/planet-client-python-3.7.9/lib/python3.7/site-packages/click/core.py", line 767, in invoke
    return __callback(*args, **kwargs)
  File "/Users/jennifer.kyle/pl/planet-client-python/planet/cli/cmds.py", line 57, in wrapper
    func(*args, **kwargs)
  File "/Users/jennifer.kyle/pl/planet-client-python/planet/cli/subscriptions.py", line 270, in request_catalog
    filter=filter)
  File "/Users/jennifer.kyle/pl/planet-client-python/planet/subscription_request.py", line 133, in catalog_source
    asset_types[i])
IndexError: list index out of range

it is fine if there is just one item-type

planet-client-python$> planet subscriptions request-catalog --item-types REScene --asset-types basic_analytic_b2 --geometry geometry.geojson --start-time 2022-02-02
{"type": "catalog", "parameters": {"item_types": ["REScene"], "asset_types": ["basic_analytic_b2"], "geometry": {"type": "Polygon", "coordinates": [[[-58.708965, -11.6562], [-58.66734, -11.6562], [-58.66734, -11.626594], [-58.708965, -11.626594], [-58.708965, -11.6562]]]}, "start_time": "2022-02-02T00:00:00Z"}}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yes I wrote this so you'd have to explicitly write out the asset type you wanted for a given item type. Sure, your example would be fine, but what if you want item_type_1, item_type_2, item_type_3, where item_type_1 and item_type_2 are both compatible with asset_type_a and item_type_3 is only compatible with asset_type_b. Would you want it to be able to parse something like

planet subscriptions request-catalog \
        --item-types item_type1,item_type_2,item_type_3 \    
        --asset-types asset_type_a, asset_type_b \
        --geometry ../jsons_for_tests/aoi.geojson \
        --start-time 2022-01-01

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see your comment in Slack. As you pointed out, the docs claim that a subscription will only be successfully made if one item_type is provided. So, as I mention and show in my comment below, I've raised a ClientError if multiple item_types are provided.

try:
asset_types[i] = specs.validate_asset_type(item_types[i],
asset_types[i])
except specs.SpecificationException as exception:
raise ClientError(exception)

parameters = {
"item_types": item_types,
"asset_types": asset_types,
Expand Down
14 changes: 9 additions & 5 deletions tests/integration/test_subscriptions_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -285,22 +285,26 @@ def test_request_base_success(invoke, geom_geojson):
assert result.exit_code == 0 # success.


def test_request_catalog_success(invoke, geom_geojson):
@pytest.mark.parametrize('item_types, asset_types',
[("PSScene", "ortho_analytic_4b_sr"),
("SkySatScene", "basic_panchromatic_dn")])
def test_request_catalog_success(invoke, geom_geojson, item_types,
asset_types):
"""Request-catalog command succeeds"""
source = {
"type": "catalog",
"parameters": {
"geometry": geom_geojson,
"start_time": "2021-03-01T00:00:00Z",
"item_types": ["PSScene"],
"asset_types": ["ortho_analytic_4b"]
"item_types": [item_types],
"asset_types": [asset_types]
}
}

result = invoke([
'request-catalog',
'--item-types=PSScene',
'--asset-types=ortho_analytic_4b',
f'--item-types={item_types}',
f'--asset-types={asset_types}',
f"--geometry={json.dumps(geom_geojson)}",
'--start-time=2021-03-01T00:00:00'
])
Expand Down
41 changes: 41 additions & 0 deletions tests/unit/test_specs.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,25 @@
'MYD09GQ',
'SkySatScene'
]
TEST_ASSET_TYPE = "basic_udm2"
ALL_SUPPORTED_ASSET_TYPES_PSSCENE = [
'ortho_analytic_8b_xml',
'basic_analytic_4b_xml',
'ortho_analytic_4b_sr',
'ortho_analytic_3b',
'basic_udm2',
'ortho_analytic_8b_sr',
'basic_analytic_8b',
'basic_analytic_4b_rpc',
'basic_analytic_8b_xml',
'ortho_udm2',
'ortho_analytic_3b_xml',
'ortho_analytic_8b',
'ortho_analytic_4b_xml',
'basic_analytic_4b',
'ortho_visual',
'ortho_analytic_4b'
]


def test_get_type_match():
Expand Down Expand Up @@ -162,3 +181,25 @@ def test_validate_supported_bundles_fail():
specs.validate_supported_bundles(TEST_ITEM_TYPE,
'analytic',
ALL_PRODUCT_BUNDLES)


def test_get_supported_assets_success():
supported_assets = specs.get_supported_assets(TEST_ITEM_TYPE)
supported_assets.sort()
ALL_SUPPORTED_ASSET_TYPES_PSSCENE.sort()
assert supported_assets == ALL_SUPPORTED_ASSET_TYPES_PSSCENE


def test_get_supported_assets_not_supported_item_type():
with pytest.raises(specs.SpecificationException):
specs.get_supported_assets('notsupported')


def test_validate_asset_type():
assert TEST_ASSET_TYPE == specs.validate_asset_type(
TEST_ITEM_TYPE, TEST_ASSET_TYPE)


def test_validate_asset_type_notsupported():
with pytest.raises(specs.SpecificationException):
specs.validate_asset_type(TEST_ITEM_TYPE, 'notsupported')
12 changes: 6 additions & 6 deletions tests/unit/test_subscription_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def test_build_request_success(geom_geojson):
"start_time": "2021-03-01T00:00:00Z",
"end_time": "2023-11-01T00:00:00Z",
"rrule": "FREQ=MONTHLY;BYMONTH=3,4,5,6,7,8,9,10",
"item_types": ["PSScene"],
"item_types": "PSScene",
"asset_types": ["ortho_analytic_4b"]
}
}
Expand Down Expand Up @@ -67,7 +67,7 @@ def test_build_request_success(geom_geojson):

def test_catalog_source_success(geom_geojson):
res = subscription_request.catalog_source(
item_types=["PSScene"],
item_types="PSScene",
asset_types=["ortho_analytic_4b"],
geometry=geom_geojson,
start_time=datetime(2021, 3, 1),
Expand All @@ -82,7 +82,7 @@ def test_catalog_source_success(geom_geojson):
"start_time": "2021-03-01T00:00:00Z",
"end_time": "2023-11-01T00:00:00Z",
"rrule": "FREQ=MONTHLY;BYMONTH=3,4,5,6,7,8,9,10",
"item_types": ["PSScene"],
"item_types": "PSScene",
"asset_types": ["ortho_analytic_4b"]
}
}
Expand All @@ -95,7 +95,7 @@ def test_catalog_source_featurecollection(featurecollection_geojson,
'''geojson specified as featurecollection is simplified down to just
the geometry'''
res = subscription_request.catalog_source(
item_types=["PSScene"],
item_types="PSScene",
asset_types=["ortho_analytic_4b"],
geometry=featurecollection_geojson,
start_time=datetime(2021, 3, 1),
Expand All @@ -106,7 +106,7 @@ def test_catalog_source_featurecollection(featurecollection_geojson,
"parameters": {
"geometry": geom_geojson,
"start_time": "2021-03-01T00:00:00Z",
"item_types": ["PSScene"],
"item_types": "PSScene",
"asset_types": ["ortho_analytic_4b"]
}
}
Expand All @@ -117,7 +117,7 @@ def test_catalog_source_featurecollection(featurecollection_geojson,
def test_catalog_source_invalid_start_time(geom_geojson):
with pytest.raises(exceptions.ClientError):
subscription_request.catalog_source(
item_types=["PSScene"],
item_types="PSScene",
asset_types=["ortho_analytic_4b"],
geometry=geom_geojson,
start_time='invalid',
Expand Down