Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
9 changes: 8 additions & 1 deletion noxfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,18 @@
nox.options.stop_on_first_error = True
nox.options.reuse_existing_virtualenvs = False

nox.options.sessions = ['lint', 'test', 'coverage', 'docs']
nox.options.sessions = ['lint', 'analyze', 'test', 'coverage', 'docs']

source_files = ("planet", "examples", "tests", "setup.py", "noxfile.py")


@nox.session
def analyze(session):
session.install(".[lint]")

session.run("mypy", "planet")


@nox.session
def coverage(session):
session.install("-e", ".[test]")
Expand Down
9 changes: 8 additions & 1 deletion planet/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,11 @@
from .auth import Auth
from .clients import DataClient, OrdersClient

__all__ = [Session, OrdersClient, order_request, reporting, Auth, DataClient]
__all__ = [
'Auth',
'DataClient'
'OrdersClient',
'order_request',
'reporting',
'Session',
]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

__all__ requires strings, not objects.

18 changes: 12 additions & 6 deletions planet/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,14 @@
BASE_URL = f'{PLANET_BASE_URL}/v0/auth'
ENV_API_KEY = 'PL_API_KEY'

AuthType = httpx.Auth
Copy link
Contributor Author

@sgillies sgillies May 4, 2022

Choose a reason for hiding this comment

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

Resolves

planet/http.py:123: error: Argument "auth" to "AsyncClient" has incompatible type "planet.auth.Auth"; expected "Union[Tuple[Union[str, bytes], Union[str, bytes]], Callable[[Request], Request], httpx._auth.Auth, None]"



class Auth(metaclass=abc.ABCMeta):
'''Handle authentication information for use with Planet APIs.'''

@staticmethod
def from_key(key: str) -> Auth:
def from_key(key: str) -> AuthType:
'''Obtain authentication from api key.

Parameters:
Expand All @@ -46,7 +48,7 @@ def from_key(key: str) -> Auth:
return auth

@staticmethod
def from_file(filename: str = None) -> Auth:
def from_file(filename: str = None) -> AuthType:
'''Create authentication from secret file.

The secret file is named `.planet.json` and is stored in the user
Expand All @@ -71,7 +73,7 @@ def from_file(filename: str = None) -> Auth:
return auth

@staticmethod
def from_env(variable_name: str = None) -> Auth:
def from_env(variable_name: str = None) -> AuthType:
'''Create authentication from environment variable.

Reads the `PL_API_KEY` environment variable
Expand All @@ -80,7 +82,7 @@ def from_env(variable_name: str = None) -> Auth:
variable_name: Alternate environment variable.
'''
variable_name = variable_name or ENV_API_KEY
api_key = os.getenv(variable_name)
api_key = os.getenv(variable_name, '')
Copy link
Contributor Author

@sgillies sgillies May 4, 2022

Choose a reason for hiding this comment

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

Resolves

planet/auth.py:85: error: Argument 1 to "APIKeyAuth" has incompatible type "Optional[str]"; expected "str"

The APIKeyAuth constructor does not take None as an argument (key: str) but we might pass it None if there's nothing in the environment. Instead, we'll pass an empty string.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a nice catch

try:
auth = APIKeyAuth(api_key)
LOGGER.debug(f'Auth set from environment variable {variable_name}')
Expand All @@ -91,7 +93,7 @@ def from_env(variable_name: str = None) -> Auth:
return auth

@staticmethod
def from_login(email: str, password: str, base_url: str = None) -> Auth:
def from_login(email: str, password: str, base_url: str = None) -> AuthType:
'''Create authentication from login email and password.

Note: To keep your password secure, the use of `getpass` is
Expand All @@ -113,14 +115,18 @@ def from_login(email: str, password: str, base_url: str = None) -> Auth:

@classmethod
@abc.abstractmethod
def from_dict(cls, data: dict) -> Auth:
def from_dict(cls, data: dict) -> AuthType:
pass

@property
@abc.abstractmethod
def value(self):
pass

@abc.abstractmethod
def to_dict(self) -> dict:
pass
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolves

planet/auth.py:132: error: "Auth" has no attribute "to_dict"


def write(self, filename: str = None):
'''Write authentication information.

Expand Down
4 changes: 2 additions & 2 deletions planet/clients/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@
from .orders import OrdersClient

__all__ = [
DataClient,
OrdersClient,
'DataClient',
'OrdersClient',
]
5 changes: 3 additions & 2 deletions planet/clients/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,11 +175,12 @@ async def create_search(self,
url = self._searches_url()

# TODO: validate item_types
request_json = {
request_json: typing.Dict[str, typing.Any] = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without this hint, mypy diagnoses request_json's type as Dict[str, Collection[Any]] because name, search_filter, and item_types are all collections, and then complains when we try to add a bool item, which is not a collection.

Copy link
Contributor

Choose a reason for hiding this comment

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

ahh interesting! good catch. I wonder if we want to make this a custom type, eg JSONType and move it over, along with AuthType to a new types module?

'name': name, 'filter': search_filter, 'item_types': item_types
}
# QUESTION: can we just set the value to True or False above?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have a test that doesn't expect __daily_email_enabled: False, but I wonder if the API would handle this?

Copy link
Contributor

Choose a reason for hiding this comment

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

The API reference claims that the type of this parameter is a boolean so I imagine it would handle True/False values. Is that the question here?

if enable_email:
request_json['__daily_email_enabled'] = True
request_json['__daily_email_enabled'] = enable_email

request = self._request(url, method='POST', json=request_json)
response = await self._do_request(request)
Expand Down
4 changes: 2 additions & 2 deletions planet/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

import httpx

from .auth import Auth
from .auth import Auth, AuthType
from . import exceptions, models
from .__version__ import __version__

Expand Down Expand Up @@ -112,7 +112,7 @@ class Session(BaseSession):
```
'''

def __init__(self, auth: Auth = None):
def __init__(self, auth: AuthType = None):
"""Initialize a Session.

Parameters:
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
'pytest', 'pytest-asyncio==0.16', 'pytest-cov', 'respx==0.16.3'
]

lint_requires = ['flake8', 'yapf']
lint_requires = ['flake8', 'mypy', 'yapf']

doc_requires = [
'mkdocs==1.3',
Expand Down