Get auth info as early as possible, store in context, use that

[sgillies]

A possible solution for one of the issues reported in #643.

In a nutshell, the root planet command is now responsible for finding auth info and planet auth value reports on the active auth, whatever the source.

[sgillies]

This now reports on the active auth object.

[sgillies]

Now the root "planet" command takes charge of auth. Previously, we deferred until calling the Session constructor.

[sgillies]

Warnings might be added here, but I think that could also be done in another PR.

[jreiberkyle]

Having planet take charge of auth is a great idea! To minimize code in the CLI wrapper and make it easier to maintain and understand default auth behavior, what about adding the logic for getting auth to the auth module, as something like Auth.default() -> AuthType then using that function here and also in the Session class.

[sgillies]

This is what deferral to the Session constructor looked like.

[jreiberkyle]

Just a drive-by question: will this change affect CLI help at all? that is, planet orders request --help won't be upset if we don't have auth specified anywhere, will it?

[sgillies]

@jreiberkyle oh yeah, that needs to be checked. Here's what I see after moving my secret file and making sure I don't have PL_API_KEY set.

$ planet auth value
Error: Auth information does not exist or is corrupted. Initialize with `planet auth init`.
$ planet orders request --help
Usage: planet orders request [OPTIONS]

  Generate an order request.

  This command provides support for building an order description used in
  creating an order. It outputs the order request, optionally pretty- printed.

Options:
  --name TEXT                     Order name. Does not need to be unique.
                                  [required]
  --bundle [analytic|analytic_udm2|analytic_3b_udm2|analytic_5b|analytic_5b_udm2|analytic_8b_udm2|visual|uncalibrated_dn|uncalibrated_dn_udm2|basic_analytic|basic_analytic_udm2|basic_analytic_8b_udm2|basic_uncalibrated_dn|basic_uncalibrated_dn_udm2|analytic_sr|analytic_sr_udm2|analytic_8b_sr_udm2|basic_uncalibrated_dn_nitf|basic_uncalibrated_dn_nitf_udm2|basic_analytic_nitf|basic_analytic_nitf_udm2|basic_panchromatic|basic_panchromatic_dn|panchromatic|panchromatic_dn|panchromatic_dn_udm2|pansharpened|pansharpened_udm2|basic_l1a_dn]
                                  Product bundle.  [required]
  --id TEXT                       One or more comma-separated item IDs.
                                  [required]
  --item-type TEXT                Specify an item type  [required]
  --clip JSON                     Clip feature GeoJSON. Can be a json string,
                                  filename, or '-' for stdin.
  --tools JSON                    Toolchain JSON. Can be a json string,
                                  filename, or '-' for stdin.
  --email                         Send email notification when order is
                                  complete.
  --cloudconfig JSON              Credentials for cloud storage provider to
                                  enable cloud delivery of data. Can be a json
                                  string, filename, or '-' for stdin.
  --stac / --no-stac              Do not request metadata to be in
                                  SpatioTemporal Asset Catalog (STAC) format.
  --pretty                        Format JSON output.
  --help                          Show this message and exit.

[jreiberkyle]

nice to be getting rid of these per-client calls to auth. I seem to remember those going in to resolve #463 so I am surprised they are no longer needed

[jreiberkyle]

In mulling over this, I can't recall why we need to manage auth information in the CLI. The priority of auth sources is the same for the CLI and API. Why not have the Session obtain default auth every time it is initialized (which will be only once per call anyway)? The logic for obtaining default auth can be a method of the auth.Auth class, and called by both http.Session and planet auth value. We should provide logging which reports the source of the auth, probably at INFO level. Then the user could obtain the auth source through planet --verbosity DEBUG ... e.g. planet --verbosity DEBUG auth value or planet --verbosity DEBUG data search ..

[sgillies]

@jreiberkyle I like to think of software as an onion (this is not my original analogy). On the outside is the filesystem and the environment, which is where our auth sources are (a secret file or an environment variable). The CLI is the outermost layer of our onion and necessarily has to interact with the filesystem and environment. This package's http module is a few layers down in our onion and shouldn't (in my ideal onion world) be interacting with the filesystem and environment. It should be getting auth information from the next layer out (a service client), which in turn gets it from the next layer out (the CLI). I've worked on projects with no such boundaries and they always suffer for the lack of them. That's why I changed the code to have the outermost layer of our software be responsible for getting the secret and passing it further down.

[jreiberkyle]

@sgillies That logic makes sense. In keeping with this approach, then, would we want to remove the logic for obtaining auth information (from env and file) from within Session as well?

[sgillies]

@jreiberkyle no, I think we should keep it. In some applications (like a lot of our usage examples and notebooks), a Session can be the outermost layer, yes? Like

async with Session() as session:
    ...

It's pragmatic to have it. But let's not use it like that in our CLI, that's all I'm saying.

[jreiberkyle]

Ahh okay I understand. It's context-dependent. That makes sense. I'm on board with this approach.

[jreiberkyle]

@sgillies for the benefit of my understanding:

here, in "a few layers down in our onion", our onion == CLI, right?

This package's http module is a few layers down in our onion and shouldn't (in my ideal onion world) be interacting with the filesystem and environment. It should be getting auth information from the next layer out (a service client), which in turn gets it from the next layer out (the CLI).

but for the API, the http module is the outermost layer of the onion.

So, for the CLI application, the CLI should be obtaining the auth info, but for the python application, it is ok for the http module to obtain the auth information - because each is the outermost layer of the onion for it's own application.

I can follow this conceptually, though it does seem confusing since the http module is the same in both applications, so we pay for this 'auth obtained at outer layer of the onion' with 'logic for obtaining auth in two places in the logic chain'. So, your experience with how this tradeoff has come out in favor of the 'outer layer' approach is helpful. Would you be willing to elaborate a little on your experience alluded to in the quote below?

I've worked on projects with no such boundaries and they always suffer for the lack of them. That's why I changed the code to have the outermost layer of our software be responsible for getting the secret and passing it further down.

[jreiberkyle]

this has been used as input to #809, see that PR for implementation