-
Notifications
You must be signed in to change notification settings - Fork 98
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
| """ | ||
| 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
| 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-01There 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 typessubscription_request.catalog_source()clients, and consiquently in its CLI commandrequest-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:
--helpchoices for the following CLI functions:subscriptions request-catalogdata search-updatedata asset-downloaddata asset-activatedata asset-waititem_typevalidation toget_asset(), which eachasset-*CLI command updated calls.asset_typevalidation insubscription_requestwhichrequest-catalogcalls.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_4bOld 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