Add client method to Session class #858

sgillies · 2023-02-26T23:59:55Z

It gets a client by the name of the client module. orders -> planet.clients.orders.OrdersClient.

The intent is to simplify the API for casual users. The API centers the Session class. It feels natural to let Session be a factory for clients. And it goes well with code autocompletion. Type session. in VS Code and we get a prompt for .client().

Type session.client( and we get a prompt to call it with a name like data, orders, or subscriptions. Actually, I think the docs could be slightly improved to help even more.

The new method is intended to be a convenience for users and documentation writers. The Session and three client classes are not otherwise changed. They don't need to be and much testing implementation hinges on the existing relationship between client classes and Session. The pattern used by OrdersClient(session) is a form of dependency injection; it really does make writing good tests easier.

The CLI commands are left as they are.

Under docs/, all examples of client = SomethingClient(session) have been changed to client = session.client('something').

Resolves #857.

It gets a client by name. Resolves #857

sgillies · 2023-02-27T16:30:16Z

docs/get-started/upgrading.md

@@ -24,7 +24,7 @@ The best way of doing this is wrapping any code that invokes a client class in a

 ```python
 async with Session() as session:


This is what I mean by "centers the Session class". Creating one is the first thing we do and it's the manager for the context of a block of code.

sgillies · 2023-02-27T16:31:20Z

planet/__init__.py

 from .io import collect

 __all__ = [
    'Auth',
    'collect',
    'DataClient'
    'OrdersClient',
+    'SubscriptionsClient',


I found that this was missing!

sgillies · 2023-02-27T16:35:50Z

planet/http.py

+        # To avoid circular dependency.
+        from planet.clients.data import DataClient
+        from planet.clients.orders import OrdersClient
+        from planet.clients.subscriptions import SubscriptionsClient


Session and clients aren't fully decoupled because the client modules import http. We could fix that in the future if it became important to do so. For now, we can tolerate some imports here. They don't have a significant impact on performance. By the time this method is called, those modules will already have been loaded.

Do you want to file a ticket for fixing that in the future?

No, not now. I'm not sure it's important. I changed the text above to reflect that.

Note: the client modules import http in order to use http.Session in their type hints.

I saw that!

sgillies · 2023-02-27T16:37:19Z

planet/http.py

+            return {
+                'data': DataClient,
+                'orders': OrdersClient,
+                'subscriptions': SubscriptionsClient


For now, I'm statically configuring the mapping between names and clients here. Getting too fancy would be a mistake. We have no plans to have multiple clients per API or support user-supplied client plugins.

Looking at my PR as if I was a reviewer, it occurs to me that this static mapping could be moved to planet/client/__init__.py. That would let us reduce those 3 imports to 1.

sgillies · 2023-02-27T16:37:47Z

tests/unit/test_session.py

+    """Get an exception when no such client exists."""
+    session = Session()
+    with pytest.raises(ClientError):
+        _ = session.client('bogus')


Test coverage of the new code is 100%.

sgillies · 2023-02-27T17:47:33Z

planet/http.py

        """Get a client by its module name.

        Parameters:
-            name: the name of the client module: data, orders, or
-                subscriptions.
+            name: one of 'data', 'orders', or 'subscriptions'.


This is pretty nice, I think. A user is told that they should pass 'data', 'orders', or 'subscription' as the argument.

This typing hint IS nice! This would be great to use this for all of the other arguments that have a set number of 'valid' entries, we already have them defined either at the top of the client module or in the specs module.

sgillies · 2023-02-27T17:48:03Z

setup.py

@@ -30,6 +30,7 @@
    'jsonschema',
    'pyjwt>=2.1',
    'tqdm>=4.56',
+    'typing-extensions',


This module provides a backport of Literal for Python 3.7.

JuLeeAtPlanet

One comment for you, but good to go!

sgillies · 2023-02-28T19:19:16Z

planet/http.py


        try:
-            return client_directory[name](self, base_url=base_url)
+            return _client_directory[name](self, base_url=base_url)


This way the Session class only needs to know that it can get a client with a name, it's not responsible for the directory. Looser coupling.

sgillies · 2023-02-28T23:50:49Z

Restoring the branch, we might be reverting this.

sgillies · 2023-02-28T23:59:08Z

Reverted pending more discussion.

jreiberkyle · 2023-03-08T16:34:22Z

This PR looks great and is ready to go. I really like the use of Literals to allow specifying valid entries. That is nice!

sgillies added 4 commits February 26, 2023 16:58

Add client method to Session class

Verified

This commit was signed with the committer’s verified signature.

nobu Nobuyoshi Nakada

GPG key ID: 3582D74E1FEE4465

Verified
Learn about vigilant mode

0b0f69b

It gets a client by name. Resolves #857

Add test for the no such client case

4691840

Add session unit tests

f1b7b65

Update docs

762345a

sgillies marked this pull request as ready for review February 27, 2023 16:26

sgillies requested review from jreiberkyle and JuLeeAtPlanet February 27, 2023 16:26

sgillies added this to the 2.0b2: Iterative improvements milestone Feb 27, 2023

sgillies commented Feb 27, 2023

View reviewed changes

Use Literal type (from Python 3.8)

1ec36f8

sgillies commented Feb 27, 2023

View reviewed changes

Add a change log entry

184f538

JuLeeAtPlanet approved these changes Feb 27, 2023

View reviewed changes

sgillies self-assigned this Feb 28, 2023

sgillies added the enhancement label Feb 28, 2023

sgillies added 2 commits February 28, 2023 12:03

Move client directory to planet/clients/__init__.py

72f1851

Make client directory private

977d51a

sgillies commented Feb 28, 2023

View reviewed changes

sgillies merged commit 1f74347 into main Feb 28, 2023

sgillies deleted the issue857 branch February 28, 2023 19:24

sgillies restored the issue857 branch February 28, 2023 23:50

sgillies mentioned this pull request Feb 28, 2023

Revert "Add client method to Session class" #860

Merged

sgillies mentioned this pull request Mar 8, 2023

Let Session get clients by name #867

Merged

This was referenced Mar 15, 2023

Release-2.0.0-rc.2 #872

Closed

Release 2.0.0-rc.2 #873

Merged

jreiberkyle deleted the issue857 branch March 15, 2023 19:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add client method to Session class #858

Add client method to Session class #858

sgillies commented Feb 26, 2023 •

edited

Loading

sgillies Feb 27, 2023

sgillies Feb 27, 2023

sgillies Feb 27, 2023 •

edited

Loading

JuLeeAtPlanet Feb 27, 2023

sgillies Feb 27, 2023

sgillies Feb 28, 2023

JuLeeAtPlanet Feb 28, 2023

sgillies Feb 27, 2023

sgillies Feb 28, 2023

sgillies Feb 27, 2023

sgillies Feb 27, 2023

jreiberkyle Feb 28, 2023

sgillies Feb 27, 2023

JuLeeAtPlanet left a comment

sgillies Feb 28, 2023

sgillies commented Feb 28, 2023

sgillies commented Feb 28, 2023

jreiberkyle commented Mar 8, 2023

		@@ -24,7 +24,7 @@ The best way of doing this is wrapping any code that invokes a client class in a

		```python
		async with Session() as session:

Add client method to Session class #858

Add client method to Session class #858

Conversation

sgillies commented Feb 26, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sgillies Feb 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JuLeeAtPlanet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sgillies commented Feb 28, 2023

sgillies commented Feb 28, 2023

jreiberkyle commented Mar 8, 2023

sgillies commented Feb 26, 2023 •

edited

Loading

sgillies Feb 27, 2023 •

edited

Loading