-
Notifications
You must be signed in to change notification settings - Fork 95
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
Conversation
TO DO:
|
…et types for a given item type.
planet/cli/data.py
Outdated
@@ -503,9 +503,9 @@ async def search_update(ctx, | |||
""" | |||
async with data_client(ctx) as cl: | |||
items = await cl.update_search(search_id, | |||
name, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Undoing this so that it can be fixed by #908. CC @jreiberkyle
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking much better, still some fixes recommended. great work!
planet/subscription_request.py
Outdated
@@ -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)): |
There was a problem hiding this comment.
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"}}
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
Now, we can validate multiple ❯ planet subscriptions request-catalog \
--item-types psscene \
--asset-types ortho_analytic_4b,basic_analytic_8b_xml \
--geometry ../jsons_for_tests/aoi.geojson \
--start-time 2022-01-01
{"type": "catalog", "parameters": {"item_types": ["psscene"], "asset_types": ["ortho_analytic_4b", "basic_analytic_8b_xml"], "geometry": {"type": "Polygon", "coordinates": [[[7.05322265625, 46.81509864599243], [7.580566406250001, 46.81509864599243], [7.580566406250001, 47.17477833929903], [7.05322265625, 47.17477833929903], [7.05322265625, 46.81509864599243]]]}, "start_time": "2022-01-01T00:00:00Z"}} As you pointed out in the docs (https://developers.planet.com/docs/subscriptions/source/#validation). So, now the SDK will fail gracefully if more than one ❯ planet subscriptions request-catalog \
--item-types psscene,psscene \
--asset-types ortho_analytic_4b,basic_analytic_8b_xml \
--geometry ../jsons_for_tests/aoi.geojson \
--start-time 2022-01-01
Error: Subscription can only be successfully created if one item type is specified. I'd love to change |
generally I agree that the term |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getting picky with tests on this one. code looks great. I'll approve here and you can merge when you make the test changes, if you agree to them.
Co-authored-by: Jennifer Reiber Kyle <jreiberkyle@users.noreply.github.com>
looks great! Great job! |
Related Issue(s):
Closes #877 and #903
Proposed Changes:
For inclusion in changelog (if applicable):
, and and item types and asset types
subscription_request.catalog_source()clients, and consiquently in its CLI command
request-catalog`. This allows case insensitivity and a list of valid options of both item types and asset types in both the Data and Subscriptions the CLI.Not intended for changelog:
--help
choices for the following CLI functions:subscriptions request-catalog
data search-update
data asset-download
data asset-activate
data asset-wait
item_type
validation toget_asset()
, which eachasset-*
CLI command updated calls.asset_type
validation insubscription_request
whichrequest-catalog
calls.Diff of User Interface
Old behavior:
New behavior:
Old behavior:
New behavior:
Old behavior:
New behavior:
Old behavior:
New behavior:
Old behavior:
New behavior:
Old behavior:
New behavior:
❯ planet data asset-activate psscene 20220101_153358_18_2421 basic_analytic_4b
Old behavior:
New behavior:
Old behavior:
New behavior:
Old behavior:
New behavior:
Old behavior:
New behavior:
Old behavior:
New behavior:
PR Checklist:
(Optional) @mentions for Notifications:
@sgillies