Skip to content

Add identification headers to SDK communication with servers #754

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 13 commits into from
Nov 10, 2022
Merged

Add identification headers to SDK communication with servers #754

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
82d0511
remove Request model
jreiberkyle Nov 5, 2022
7902cd6
create requests with Session class, provides merging of headers
jreiberkyle Nov 5, 2022
fdf9c12
Paged initiated with a response instead of json
jreiberkyle Nov 8, 2022
0fb1ea1
use session.request directly instead of client method
jreiberkyle Nov 8, 2022
b8f23be
update data client to use session.request directly
jreiberkyle Nov 8, 2022
9b971f2
update subscriptions client to have session be a private property and…
jreiberkyle Nov 9, 2022
cb74136
remove empty Request class placeholder because the class is no longer…
jreiberkyle Nov 9, 2022
b2b86d0
Merge branch 'main' into headers-fix-739
jreiberkyle Nov 9, 2022
becfe13
type hinting fixes
jreiberkyle Nov 9, 2022
0f62a90
remove unused portions of tests
jreiberkyle Nov 9, 2022
1e78a7b
add test for request headers and content
jreiberkyle Nov 9, 2022
e15bfc7
add test for identifying headers
jreiberkyle Nov 10, 2022
2a13a07
fix create_order not sending data with the request and add a test for…
jreiberkyle Nov 10, 2022
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
8 changes: 3 additions & 5 deletions planet/auth.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
import httpx
import jwt

from . import http, models
from . import http
from .constants import PLANET_BASE_URL, SECRET_FILE_PATH
from .exceptions import AuthException
from typing import Optional
Expand Down Expand Up @@ -177,10 +177,8 @@ def login(self, email: str, password: str) -> dict:
data = {'email': email, 'password': password}

sess = http.AuthSession()
req = models.Request(url, method='POST', json=data)
resp = sess.request(req)
auth_data = self.decode_response(resp)
return auth_data
resp = sess.request(url=url, method='POST', json=data)
return self.decode_response(resp)

@staticmethod
def decode_response(response):
Expand Down
76 changes: 32 additions & 44 deletions planet/clients/data.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from .. import exceptions
from ..constants import PLANET_BASE_URL
from ..http import Session
from ..models import Paged, Request, Response, StreamingBody
from ..models import Paged, StreamingBody

BASE_URL = f'{PLANET_BASE_URL}/data/v1/'
SEARCHES_PATH = '/searches'
Expand Down Expand Up @@ -93,17 +93,6 @@ def _searches_url(self):
def _item_url(self, item_type, item_id):
return f'{self._base_url}/item-types/{item_type}/items/{item_id}'

def _request(self, url, method, data=None, params=None, json=None):
return Request(url, method=method, data=data, params=params, json=json)

async def _do_request(self, request: Request) -> Response:
"""Submit a request and get response.

Parameters:
request: request to submit
"""
return await self._session.request(request)

async def search(self,
item_types: List[str],
search_filter: dict,
Expand Down Expand Up @@ -185,11 +174,11 @@ async def search(self,
f'{sort} must be one of {SEARCH_SORT}')
params['_sort'] = sort

request = self._request(url,
method='POST',
json=request_json,
params=params)
return Items(request, self._do_request, limit=limit)
response = await self._session.request(method='POST',
url=url,
json=request_json,
params=params)
return Items(response, self._session.request, limit=limit)

async def create_search(self,
name: str,
Expand Down Expand Up @@ -229,15 +218,16 @@ async def create_search(self,
url = self._searches_url()

# TODO: validate item_types
request_json = {
request = {
'name': name,
'filter': search_filter,
'item_types': item_types,
'__daily_email_enabled': enable_email
}

request = self._request(url, method='POST', json=request_json)
response = await self._do_request(request)
response = await self._session.request(method='POST',
url=url,
json=request)
return response.json()

async def update_search(self,
Expand All @@ -260,15 +250,16 @@ async def update_search(self,
"""
url = f'{self._searches_url()}/{search_id}'

request_json = {
request = {
'name': name,
'filter': search_filter,
'item_types': item_types,
'__daily_email_enabled': enable_email
}

request = self._request(url, method='PUT', json=request_json)
response = await self._do_request(request)
response = await self._session.request(method='PUT',
url=url,
json=request)
Copy link
Contributor

@sgillies sgillies Nov 10, 2022

Choose a reason for hiding this comment

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

@jreiberkyle @kevinlacaille tangential comment here: the code might be more readable if we didn't use "request" as both a verb and a noun. Maybe self._session.request could be changed to self._session.send someday?

Copy link
Contributor Author

@jreiberkyle jreiberkyle Nov 10, 2022

Choose a reason for hiding this comment

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

Ohh yeah I fully agree here! Right now Session has two methods that communicate with the servers: request() and stream(). Both of these suffer from verb/noun ambiguity. This was some of the satisfaction in removing the Request and Stream classes. At least we no longer have noun representation in the code base.

return response.json()

async def list_searches(self,
Expand Down Expand Up @@ -307,8 +298,9 @@ async def list_searches(self,
f'{search_type} must be one of {LIST_SEARCH_TYPE}')

url = f'{self._searches_url()}'
request = self._request(url, method='GET')
return Searches(request, self._do_request, limit=limit)

response = await self._session.request(method='GET', url=url)
return Searches(response, self._session.request, limit=limit)

async def delete_search(self, search_id: str):
"""Delete an existing saved search.
Expand All @@ -321,8 +313,7 @@ async def delete_search(self, search_id: str):
"""
url = f'{self._searches_url()}/{search_id}'

request = self._request(url, method='DELETE')
await self._do_request(request)
await self._session.request(method='DELETE', url=url)

async def get_search(self, search_id: str) -> dict:
"""Get a saved search by id.
Expand All @@ -337,9 +328,9 @@ async def get_search(self, search_id: str) -> dict:
planet.exceptions.APIError: On API error.
"""
url = f'{self._searches_url()}/{search_id}'
req = self._request(url, method='GET')
resp = await self._do_request(req)
return resp.json()

response = await self._session.request(method='GET', url=url)
return response.json()

async def run_search(self,
search_id: str,
Expand All @@ -359,8 +350,8 @@ async def run_search(self,
"""
url = f'{self._searches_url()}/{search_id}/results'

request = self._request(url, method='GET')
return Items(request, self._do_request, limit=limit)
response = await self._session.request(method='GET', url=url)
return Items(response, self._session.request, limit=limit)

async def get_stats(self,
item_types: List[str],
Expand Down Expand Up @@ -388,14 +379,15 @@ async def get_stats(self,

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

request_json = {
request = {
'interval': interval,
'filter': search_filter,
'item_types': item_types
}

request = self._request(url, method='POST', json=request_json)
response = await self._do_request(request)
response = await self._session.request(method='POST',
url=url,
json=request)
return response.json()

async def list_asset_types(self) -> List[dict]:
Expand Down Expand Up @@ -500,8 +492,8 @@ async def list_item_assets(self, item_type_id: str, item_id: str) -> dict:
planet.exceptions.APIError: On API error.
"""
url = f'{self._item_url(item_type_id, item_id)}/assets'
request = self._request(url, method='GET')
response = await self._do_request(request)

response = await self._session.request(method='GET', url=url)
return response.json()

async def get_asset(self,
Expand Down Expand Up @@ -558,9 +550,8 @@ async def activate_asset(self, asset: dict):

# lets not try to activate an asset already activating or active
if status == 'inactive':
request = self._request(url, method='GET')
# no response is returned
await self._do_request(request)
await self._session.request(method='GET', url=url)

return

Expand Down Expand Up @@ -623,8 +614,7 @@ async def wait_asset(
raise exceptions.ClientError(
'asset missing ["_links"]["_self"] entry.')

request = self._request(asset_url, method='GET')
response = await self._do_request(request)
response = await self._session.request(method='GET', url=asset_url)
asset = response.json()

if max_attempts and num_attempts >= max_attempts:
Expand Down Expand Up @@ -669,9 +659,7 @@ async def download_asset(self,
raise exceptions.ClientError(
'asset missing ["location"] entry. Is asset active?')

req = self._request(location, method='GET')

async with self._session.stream(req) as resp:
async with self._session.stream(method='GET', url=location) as resp:
body = StreamingBody(resp)
dl_path = Path(directory, filename or body.name)
dl_path.parent.mkdir(exist_ok=True, parents=True)
Expand Down
54 changes: 21 additions & 33 deletions planet/clients/orders.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
from .. import exceptions
from ..constants import PLANET_BASE_URL
from ..http import Session
from ..models import Paged, Request, Response, StreamingBody
from ..models import Paged, StreamingBody

BASE_URL = f'{PLANET_BASE_URL}/compute/ops'
STATS_PATH = '/stats/orders/v2'
Expand Down Expand Up @@ -112,17 +112,6 @@ def _orders_url(self):
def _stats_url(self):
return f'{self._base_url}{STATS_PATH}'

def _request(self, url, method, data=None, params=None, json=None):
return Request(url, method=method, data=data, params=params, json=json)

async def _do_request(self, request: Request) -> Response:
'''Submit a request and get response.

Parameters:
request: request to submit
'''
return await self._session.request(request)

async def create_order(self, request: dict) -> dict:
'''Create an order request.

Expand Down Expand Up @@ -156,10 +145,10 @@ async def create_order(self, request: dict) -> dict:
planet.exceptions.APIError: On API error.
'''
url = self._orders_url()

req = self._request(url, method='POST', json=request)
resp = await self._do_request(req)
return resp.json()
response = await self._session.request(method='POST',
url=url,
json=request)
return response.json()

async def get_order(self, order_id: str) -> dict:
'''Get order details by Order ID.
Expand All @@ -177,9 +166,8 @@ async def get_order(self, order_id: str) -> dict:
self._check_order_id(order_id)
url = f'{self._orders_url()}/{order_id}'

req = self._request(url, method='GET')
resp = await self._do_request(req)
return resp.json()
response = await self._session.request(method='GET', url=url)
return response.json()

async def cancel_order(self, order_id: str) -> dict:
'''Cancel a queued order.
Expand All @@ -197,9 +185,8 @@ async def cancel_order(self, order_id: str) -> dict:
self._check_order_id(order_id)
url = f'{self._orders_url()}/{order_id}'

req = self._request(url, method='PUT')
resp = await self._do_request(req)
return resp.json()
response = await self._session.request(method='PUT', url=url)
return response.json()

async def cancel_orders(self,
order_ids: Optional[List[str]] = None) -> dict:
Expand All @@ -224,9 +211,11 @@ async def cancel_orders(self,
self._check_order_id(oid)
cancel_body['order_ids'] = order_ids

req = self._request(url, method='POST', json=cancel_body)
resp = await self._do_request(req)
return resp.json()
response = await self._session.request(method='POST',
url=url,
json=cancel_body)

return response.json()

async def aggregated_order_stats(self) -> dict:
'''Get aggregated counts of active orders.
Expand All @@ -238,9 +227,8 @@ async def aggregated_order_stats(self) -> dict:
planet.exceptions.APIError: On API error.
'''
url = self._stats_url()
req = self._request(url, method='GET')
resp = await self._do_request(req)
return resp.json()
response = await self._session.request(method='GET', url=url)
return response.json()

async def download_asset(self,
location: str,
Expand All @@ -264,9 +252,7 @@ async def download_asset(self,
Raises:
planet.exceptions.APIError: On API error.
"""
req = self._request(location, method='GET')

async with self._session.stream(req) as resp:
async with self._session.stream(method='GET', url=location) as resp:
body = StreamingBody(resp)
dl_path = Path(directory, filename or body.name)
dl_path.parent.mkdir(exist_ok=True, parents=True)
Expand Down Expand Up @@ -505,5 +491,7 @@ async def list_orders(self,
else:
params = None

request = self._request(url, 'GET', params=params)
return Orders(request, self._do_request, limit=limit)
response = await self._session.request(method='GET',
url=url,
params=params)
return Orders(response, self._session.request, limit=limit)
Loading