Refactor Authorization docs (leaving stub pages behind) #1046

sirosen · 2024-09-06T19:36:52Z

Move authorizer documentation into authorization/
Move scope docs into authorization/

This is a preparatory move serving three purposes.
(1) It's just good cleanup.
(2) The new structure will let us document the consents module in a logical place when it moves out of experimental.
(3) It makes room for GlobusApp, new tokenstorage docs, and other improvements.

📚 Documentation preview 📚: https://globus-sdk-python--1046.org.readthedocs.build/en/1046/

- Move `authorization.rst` to `authorization/globus_authorizers.rst` - Create a stub at `authorization.rst` (orphan doc) which directs users to the new page - Introduce `authorization/index.rst` with a single-item TOC

- Move `scopes.rst` to `authorization/scopes_and_consents/scopes.rst` - Add `scopes.rst` as a stub, pointing to the new page - Reduce `experimental/scope_parser.rst` to a stub (was full doc!) - Move `MutableScope` docs to a separate page in `scopes_and_consents/` and note it as deprecated (doc note only) - Update `MutableScope` usage docs to use `Scope` instead - Update toctrees to include the new docs

docs/authorization/globus_authorizers.rst

derek-globus

I don't know how much of this copy is new vs copy/pasted into a new location.

I like the new structure of it a lot but think that the page text itself needs some massaging to get to a consumable state. In the globus authorizers page in particular, we should structure to be as approachable as possible; there's a good chance, this is the first time that users are interacting with globus authorization.

docs/authorization/globus_authorizers.rst

derek-globus · 2024-09-09T17:11:41Z

docs/authorization/scopes_and_consents/index.rst

+.. toctree::
+    :maxdepth: 1
+
+    scopes
+    mutable_scopes


Move this to the top of the page (or signal in a more apparent way that this is a nav link structure).

Otherwise it looks at a glance to me like "these docs have been moved to docs.globus.org/guides/overview/clients_scopes_and_consents."

Yeah, I'll rearrange. I don't know exactly how yet; let me get in there and see what looks right.

I broke things apart into sections, so that the overview/background on scopes with the docs.globus.org link could go in a separate section. Take a look and LMK if we can mark this resolved!

docs/authorization/scopes_and_consents/mutable_scopes.rst

docs/authorization/scopes_and_consents/scopes.rst

derek-globus · 2024-09-09T17:23:23Z

docs/authorization/scopes_and_consents/scopes.rst

+In order to support optional and dependent scopes, an additional type is
+provided by ``globus_sdk.scopes``: the ``Scope`` class.


I wouldn't frame the initial reference to Scopes as "for the purpose of optional and dependent scopes". I'd just say "this is the python class model for a scope" (and maybe that it happens to support optional and dependent scopes).

Good callout! That was adapted from text we have today about MutableScope.

I've reworded, not only as you suggested but also to draw the connection that having such a model is useful for representing and manipulating dependent scopes. Take a look and see if it's satisfactory -- I think you'll agree that it's an improvement but maybe we need to refine it further.

derek-globus · 2024-09-09T17:25:07Z

docs/authorization/scopes_and_consents/scopes.rst

+.. code-block:: pycon
+
+    >>> from globus_sdk.scopes import Scope
+    >>> foo = Scope("foo")
+    >>> bar = Scope("bar")
+    >>> bar.add_dependency("baz")
+    >>> foo.add_dependency(bar)
+    >>> print(str(foo))
+    foo[bar[baz]]
+    >>> print(bar.serialize())
+    bar[baz]
+    >>> alpha = Scope("alpha")
+    >>> alpha.add_dependency("beta", optional=True)
+    >>> print(str(alpha))
+    alpha[*beta]
+    >>> print(repr(alpha))
+    Scope("alpha", dependencies=[Scope("beta", optional=True)])


Can we use python instead of pycon for this code block for consistency across the page? Or is there an important reason to affiliate this with anaconda?

pycon is Python Console. It's more appropriate for this case because it contains output.

derek-globus · 2024-09-09T17:28:07Z

docs/authorization/scopes_and_consents/scopes.rst

+.. autodata:: globus_sdk.scopes.data.AuthScopes
+    :annotation:
+
+.. autodata:: globus_sdk.scopes.data.FlowsScopes
+    :annotation:
+
+.. autodata:: globus_sdk.scopes.data.GroupsScopes
+    :annotation:
+
+.. autodata:: globus_sdk.scopes.data.NexusScopes
+    :annotation:
+
+.. autodata:: globus_sdk.scopes.data.SearchScopes
+    :annotation:
+
+.. note::
+
+    ``TimersScopes`` is also available under the legacy name ``TimerScopes``.
+
+.. autodata:: globus_sdk.scopes.data.TimersScopes
+    :annotation:
+
+.. autodata:: globus_sdk.scopes.data.TransferScopes
+    :annotation:


These constants don't actually document the piece that I'd expect users to care about looking up.

The associated documentation for all of them is a duplicate (probably inherited) docstring & parameter reference. Could we instead list out the specific instance attributes here? e.g.:

class TransferScopes all: Perform any transfer operation.

If we make that change, I'd also advocate that we move this up as far as we feel comfortable in the page; that seems like the most pertinent information (besides potential "how to use scopes" in this page) and right now it's at the bottom.

Can we save debate about this for future changes? This is a lift and shift of existing docs which are built via the custom sphinx plugin. Changing this extensively is something I'm open to doing, but want to keep out of this scope of work.

derek-globus · 2024-09-09T17:37:07Z

docs/authorization/globus_authorizers.rst

+Globus SDK clients need credentials (tokens) to authenticate against services
+and authorize their various API calls. Using Globus Auth, the Globus OAuth2
+service, clients can be provided with credentials which are either static or
+dynamic.
+
+The interface used to handle these credentials is :class:`GlobusAuthorizer`.
+:class:`GlobusAuthorizer` defines methods which provide an ``Authorization``
+header for HTTP requests, and different implementations handle Refresh Tokens,
+Access Tokens, Client Credentials, and their various usage modes.
+
+A :class:`GlobusAuthorizer` is an object which defines the following two
+operations:
+
+- get an ``Authorization`` header
+- handle a 401 Unauthorized error
+
+Clients contain authorizers, and use them to get and refresh their credentials.
+Whenever using the :ref:`Service Clients <services>`, you should be passing in an
+authorizer when you create a new client unless otherwise specified.
+
+The type of authorizer you will use depends very much on your application, but
+if you want examples you should look at the :ref:`examples section
+<examples-authorization>`.
+It may help to start with the examples and come back to the reference
+documentation afterwards.


This intro section contains too much detail.

All that needs to be conveyed (imo) is:

All service clients use credentials (globus-auth tokens) to make requests.

GlobusAuthorizer is an interface we added to make it simpler to do that right.

[Maybe] A call to action of "read on where we explain how we do that and how to use it!"

An intro section should be as concise as possible, with as little technical detail as possible. The technical detail goes into the well defined section below where readers can read it.

I disagree on some specifics, but I generally agree that this section is too long. However, as above, this is a lift and shift with minimal edits, and I would like to keep it as such. I think a lot of this content needs to go somewhere and I would not be satisfied to simply remove it from the intro without finding an appropriate location for it.

Are you proposing a specific new place for that narrative documentation? I didn't move it because I didn't want to debate this element. If there's a place we can agree upon, I'm happy to move it. Otherwise, let's leave this in as is for the moment.

Co-authored-by: derek-globus <113056046+derek-globus@users.noreply.github.com>

for more information, see https://pre-commit.ci

Per conversation, basic requirements met.

ada-globus

I'm marking as "approved" to indicate my overall endorsement of the restructuring. I have a few modest change requests, but if you're okay with those suggestions, this puts you in a position to just apply and merge so we can keep things moving.

docs/authorization/scopes_and_consents/scopes.rst

Co-authored-by: Ada <107940310+ada-globus@users.noreply.github.com>

sirosen added 2 commits September 6, 2024 13:42

Move authorizer documentation into authorization/

63e06d2

- Move `authorization.rst` to `authorization/globus_authorizers.rst` - Create a stub at `authorization.rst` (orphan doc) which directs users to the new page - Introduce `authorization/index.rst` with a single-item TOC

sirosen added the no-news-is-good-news This change does not require a news file label Sep 6, 2024

sirosen requested review from aaschaer, ada-globus, derek-globus and kurtmckee as code owners September 6, 2024 19:36

derek-globus reviewed Sep 9, 2024

View reviewed changes

docs/authorization/globus_authorizers.rst Outdated Show resolved Hide resolved

derek-globus previously requested changes Sep 9, 2024

View reviewed changes

sirosen and others added 3 commits September 9, 2024 15:05

Apply suggestions from code review

b2686c5

Co-authored-by: derek-globus <113056046+derek-globus@users.noreply.github.com>

[pre-commit.ci] auto fixes from pre-commit.com hooks

9ef085f

for more information, see https://pre-commit.ci

Changes per review

9116758

sirosen mentioned this pull request Sep 10, 2024

Promote the consents module from experimental to globus_sdk.scopes.consents #1047

Merged

ada-globus approved these changes Sep 13, 2024

View reviewed changes

Apply suggestions from code review

b3268e0

Co-authored-by: Ada <107940310+ada-globus@users.noreply.github.com>

sirosen merged commit 0c892dd into globus:main Sep 14, 2024
15 checks passed

sirosen deleted the refactor-authz-docs branch September 14, 2024 03:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor Authorization docs (leaving stub pages behind) #1046

Refactor Authorization docs (leaving stub pages behind) #1046

sirosen commented Sep 6, 2024 •

edited by github-actions bot

Loading

derek-globus left a comment

derek-globus Sep 9, 2024

sirosen Sep 9, 2024

sirosen Sep 10, 2024

derek-globus Sep 9, 2024

sirosen Sep 9, 2024

sirosen Sep 10, 2024

derek-globus Sep 9, 2024

sirosen Sep 9, 2024

derek-globus Sep 9, 2024

derek-globus Sep 9, 2024

sirosen Sep 9, 2024

derek-globus Sep 9, 2024

sirosen Sep 9, 2024

ada-globus left a comment

		In order to support optional and dependent scopes, an additional type is
		provided by ``globus_sdk.scopes``: the ``Scope`` class.

Refactor Authorization docs (leaving stub pages behind) #1046

Refactor Authorization docs (leaving stub pages behind) #1046

Conversation

sirosen commented Sep 6, 2024 • edited by github-actions bot Loading

derek-globus left a 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

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

ada-globus left a comment

Choose a reason for hiding this comment

sirosen commented Sep 6, 2024 •

edited by github-actions bot

Loading