Fix bug with str(OAuthDependentTokenResponse)

[sirosen]

The stringifier inherited from OAuthTokenResponse assumes that self.get(...) is safe, but that's only true when the data is a dict, as it normally is.
OAuthDependentTokenResponse has self.data: list. Therefore, this needs a check and special handling in the stringifier.

Add a test case which fails without this addition, then make the get() call conditional on the type of the underlying data.

Arguably this is a "bad" division of responsibilities (base class knows about its subclasses), but any alternative which solves this generically is more verbose or touches on core codepaths like the central GlobusHTTPResponse.get implementation.

[derek-globus]

It feels boundary breaking to have this check self.data but then retrieve a value by calling self.get(...)

Could we either

1. If we're comfortable that sometimes data in OAuthTokenResponse won't be a dict, make .get safe for OAuthTokenResponse. Maybe override the get method like

   class OAuthTokenReponse(GlobusHTTPResponse):
        ...
        def __getitem__(self, key: t.Union[str, int, slice])) -> t.Any:
            if not isinstance(self.data, dict):
               return None
           return super().__getitem__(key)
        ...
   

2. Or check the same thing we're querying from:
   1. Move self.get(...) to self.data.get(...) so that we're querying the same object we just verified, not relying on the fact that the internals of __getitem__ do that
   2. I guess error handling would be the way to handle the "not-a-dict" option from the response interface level

      try:
           id_token_to_print = t.cast(t.Optional[str], self.get("id_token"))
      except ValueError:
           id_token_to_print = None
      

[sirosen]

Thanks for identify this -- I didn't love it either but didn't have better ideas at the time. Why don't we go with redefining get on this one class to handle non-dict data? I'll put together a version of that later.

[sirosen]

I opened this up in order to make the change today, and I realized that there's very little reason for GlobusHTTPResponse.get not to handle list data nicely.

I've gone ahead and added a commit which moves the check to the base class' get method, and removes this check here entirely. It also adds an appropriate changelog item.

It's a pretty significant change in where the fix is applied, and it's breadth of potential impact and utility. But I have started to think that it's the best way to fix this problem.

[aaschaer]

Looks good to me!