updates from beta feedback #1148
Conversation
There was a problem hiding this comment.
I've left a few comments on the doc updates, though I had a bit of trouble following the flow of everything without the structure of the mkdocs site. I think it might be easier to review after it's been deployed.
All the actual functional changes look good to me.
| OAuth2 defines many different ways to obtain access tokens, and a full discussion | ||
| is beyond the scope of this SDK user guide. Please refer to the [Resources](#resources) | ||
| below for more information. Planet broadly divides OAuth2 use cases into | ||
| user-interactive and machine-to-machine use cases, as described in this guide. |
There was a problem hiding this comment.
In line with the 3 bullets in the prior section, it might be nice to have separate links here to the applicable auth-dev-* subsections for user access and M2M access, especially since the headings of those subsections don't quite line up 1-1 with the bullets, e.g. OAuth2 user access tokens vs ****
There was a problem hiding this comment.
I like the idea. The current example doc organization in the "auth-dev" pages do not align to this axis of auth protocol flavor. Rather, those (mostly) ladder from least to most complex for the client developer, with API keys somewhat shoved to the bottom.
I'll see what links and org I can change that makes sense.
There was a problem hiding this comment.
I think I have this in the next set of edits. (Not yet pushed)
docs/auth/auth-sdk.md
Outdated
| - Highest priority is given to command-line arguments. This applies | ||
| only to `planet` CLI use, since programmatic use of the SDK bypasses the CLI program. | ||
| Configuration persisted by the `planet` CLI is saved to configuration files | ||
| (below). |
There was a problem hiding this comment.
This could do with some rewording since the section is more SDK focused. Something along the lines of:
Highest priority is given to arguments passed to the Auth class (when using the SDK) or via the command line (when using the CLI)
Or maybe just change the L1 heading if you're meaning for this to be equally applicable to both? I believe we use "Planet Python Client" (or just "Planet Client") as the collective term for both SDK + CLI.
There was a problem hiding this comment.
I'll take your wording.
| and applications built using the SDK. | ||
|
|
||
| Applications built using the SDK may be configured to bypass home directory | ||
| profile and session storage, if this better suits the needs of the application. |
There was a problem hiding this comment.
Might be nice to link out to how to do this from here.
There was a problem hiding this comment.
Done in next push.
| |-----------------------------|-------------------------------------------------------------------------------------------------------------------------------| | ||
| | **`PL_AUTH_PROFILE`** | Specify a custom CLI managed auth client profile by name. This must name a valid CLI managed profile or an error will occur. | | ||
| | **`PL_AUTH_CLIENT_ID`** | Specify an OAuth2 M2M client ID. `PL_AUTH_CLIENT_SECRET` must also be specified, or this will be ignored. | | ||
| | **`PL_AUTH_CLIENT_SECRET`** | Specify an OAuth2 M2M client secret. `PL_AUTH_CLIENT_ID` must also be specified, or this will be ignored. | |
There was a problem hiding this comment.
Do we link to how to obtain these anywhere? I saw the link for API key instructions above, but not one for M2M clients (I know this is kinda under-construction so of this is an add it later thing, that's fine)
There was a problem hiding this comment.
Yes, I have links a couple places now.
There is a couple different tag-teams going on now: I want to link back to docs.planet.com for some things that are not SDK specific (like client registration), and at least two things are in a "under construction" state: PL APIs do not actually accept M2M tokens yet (SH APIs do). We have no self-serve process for registering interactive clients, despite the relative verbosity space that case occupies in the guide (That's because it's just more complex.)
| @@ -1,377 +1,8 @@ | |||
| # Client Authentication Guide | |||
There was a problem hiding this comment.
I was viewing this raw to get a better handle on what was left after the refactoring, and noticed the title format is different than the other files under this dir, e.g.:
---
title: Python SDK API Reference
---
Not sure if that needs to be fixed?
There was a problem hiding this comment.
I noticed that one. I don't know where that copy pasta came from.
I already removed that from a later draft, however.
docs/python/sdk-client-auth.md
Outdated
| "Confidential" OAuth2 client configurations, and needs to be documented | ||
| here. | ||
| See [Client Authentication Overview](../../auth/auth-overview) for a general | ||
| overview, and [Authentication with the SDK](../../auth/auth-sdk) for a primer |
There was a problem hiding this comment.
I know it's a little redundant, but we should probably spell out an overview of what, e.g.
"for a general overview of authentication with the Planet Client in general"
There was a problem hiding this comment.
Good point. Will adjust.
…ls to specific implementaiton examples.
Improvements from beta feedback