Skip to content

Add SkySatVideo as a valid data api item type #959

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.

Sign up for GitHub

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 3 commits into from
May 19, 2023
Merged

Add SkySatVideo as a valid data api item type #959

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e95ab06
separate data api item type validation from orders, add skysatvideo a…
jreiberkyle May 18, 2023
a5a635a
add skysatvideo to list of valid item types in data search help
jreiberkyle May 18, 2023
eabe984
minimize lines of code, update docstrings, add reference to long term…
jreiberkyle May 19, 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: 6 additions & 6 deletions planet/cli/data.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@
SEARCH_SORT_DEFAULT,
STATS_INTERVAL)

from planet.specs import (get_item_types,
validate_item_type,
from planet.specs import (get_data_item_types,
validate_data_item_type,
SpecificationException)

from . import types
Expand All @@ -38,8 +38,8 @@
from .options import limit, pretty
from .session import CliSession

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


@asynccontextmanager
Expand Down Expand Up @@ -75,7 +75,7 @@ def check_item_types(ctx, param, item_types) -> Optional[List[dict]]:
item types.'''
try:
for item_type in item_types:
validate_item_type(item_type)
validate_data_item_type(item_type)
return item_types
except SpecificationException as e:
raise click.BadParameter(str(e))
Expand All @@ -85,7 +85,7 @@ def check_item_type(ctx, param, item_type) -> Optional[List[dict]]:
'''Validates the item type provided by comparing it to all supported
item types.'''
try:
validate_item_type(item_type)
validate_data_item_type(item_type)
except SpecificationException as e:
raise click.BadParameter(str(e))

Expand Down
12 changes: 6 additions & 6 deletions planet/clients/data.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
from ..constants import PLANET_BASE_URL
from ..http import Session
from ..models import Paged, StreamingBody
from ..specs import validate_item_type
from ..specs import validate_data_item_type

BASE_URL = f'{PLANET_BASE_URL}/data/v1/'
SEARCHES_PATH = '/searches'
Expand Down Expand Up @@ -147,7 +147,7 @@ async def search(self,

search_filter = search_filter or empty_filter()

item_types = [validate_item_type(item) for item in item_types]
item_types = [validate_data_item_type(item) for item in item_types]
request_json = {'filter': search_filter, 'item_types': item_types}
if name:
request_json['name'] = name
Expand Down Expand Up @@ -204,7 +204,7 @@ async def create_search(self,
"""
url = self._searches_url()

item_types = [validate_item_type(item) for item in item_types]
item_types = [validate_data_item_type(item) for item in item_types]
request = {
'name': name,
'filter': search_filter,
Expand Down Expand Up @@ -237,7 +237,7 @@ async def update_search(self,
"""
url = f'{self._searches_url()}/{search_id}'

item_types = [validate_item_type(item) for item in item_types]
item_types = [validate_data_item_type(item) for item in item_types]
request = {
'name': name,
'filter': search_filter,
Expand Down Expand Up @@ -396,7 +396,7 @@ async def get_stats(self,

url = f'{self._base_url}{STATS_PATH}'

item_types = [validate_item_type(item) for item in item_types]
item_types = [validate_data_item_type(item) for item in item_types]
request = {
'interval': interval,
'filter': search_filter,
Expand Down Expand Up @@ -450,7 +450,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)
item_type_id = validate_data_item_type(item_type_id)
assets = await self.list_item_assets(item_type_id, item_id)

try:
Expand Down
11 changes: 11 additions & 0 deletions planet/specs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,17 @@ def validate_item_type(item_type):
return _validate_field(item_type, supported_item_types, 'item_type')


def validate_data_item_type(item_type):
'''Validate and correct capitalization of data api item type.'''
return _validate_field(item_type, get_data_item_types(), 'item_type')
Copy link
Contributor

@sgillies sgillies May 18, 2023

Choose a reason for hiding this comment

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

@jreiberkyle since get_data_item_types() below is essentially a one line function, how about this instead? Let's avoid creating new functions when they're not strictly required at the moment.

Suggested change
return _validate_field(item_type, get_data_item_types(), 'item_type')
return _validate_field(item_type, get_item_types() | {'SkySatVideo'}, 'item_type')

Copy link
Contributor Author

@jreiberkyle jreiberkyle May 19, 2023

Choose a reason for hiding this comment

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

ohhh wait, I was going to say yes absolutely but the data CLI uses get_data_item_types() to list the valid item types in the help. So removing get_data_item_types() would require the same one-liner to be added to planet/cli/data.py as well. Introducing get_date_item_types() was chosen to keep the "hacktastic" code in one place within specs.

Copy link
Contributor Author

@jreiberkyle jreiberkyle May 19, 2023

Choose a reason for hiding this comment

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

but looking at the code, there are a lot of opportunities to get rid of short-term variables and extra lines of code. I did what I could while keeping get_data_item_types()



def get_data_item_types():
'''Item types supported by the data api.'''
# This is a quick-fix for gh-956, to be superseded by gh-960
return get_item_types() | {'SkySatVideo'}


def validate_order_type(order_type):
return _validate_field(order_type, SUPPORTED_ORDER_TYPES, 'order_type')

Expand Down
5 changes: 5 additions & 0 deletions tests/unit/test_specs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,11 @@ def test_validate_item_type_notsupported_itemtype():
specs.validate_item_type('notsupported')


def test_validate_data_item_type():
'''ensure skysatvideo is included'''
specs.validate_data_item_type('skysatvideo')


def test_validate_order_type_supported():
assert 'full' == specs.validate_order_type('FULL')

Expand Down