Implement the X-Globus-Client-Info header as a feature of transport objects #990
Conversation
sirosen
requested review from
aaschaer,
ada-globus,
derek-globus and
kurtmckee
as code owners
|
CI is failing. |
This is a simple implementation of the spec as a dedicated object. It serializes into headers when requests are sent. A default implementation is provided which seeds the data with the SDK product and version info.
- Add a test parser to the testsuite - Add a test which validates that this parser picks up on the SDK-provided data (with no errors) and can handle added data
- Move narrative doc from module docstring into the relevant class docstring, for easier inclusion in our HTML docs. - Enhance said narrative doc to better explain the header in a user-facing manner, and ensure correct ReST formatting. - Add a changelog which minimally explains the feature.
sirosen
force-pushed
the
globus-clientinfo
branch
from
5d162c7
to
31a3c81
Compare
|
I made a change (as a fixup, so you can't see it in the history) to switch from I've just fixup'ed my fixup and pushed, so the state should be clean now. |
|
|
||
| def __init__(self) -> None: | ||
| super().__init__() | ||
| self.add({"product": "python-sdk", "version": __version__}) |
There was a problem hiding this comment.
It seems like this information should always be tacked on at serialization time.
That is to say: devs who are using the SDK will always have critical SDK client information present.
Therefore I think that there should be no DefaultGlobusClientInfo class, but instead the SDK client information should be hard-coded to serialize into the HTTP header (and, probably, be the first client info in the serialized header, in case the remainder of the header is b0rked for any reason and cannot be deserialized).
There was a problem hiding this comment.
Okay, let's merge it into the superclass so that it's part of the base implementation here. I buy a lot of the thinking here.
The place where I want to exercise some caution is around baking it in so deeply that removing, inspecting, or manipulating it requires subclassing the transport class itself. I'd therefore prefer to keep it as structured data, but structured data which is populated by default.
- Coalesce into a single class (rather than base+default) - Fix various typos around the header spelling - Rename the transport attribute to `globus_client_info` - Update tests to match implementation updates - Update docs to match implementation updates
Why
The goal of this PR is to start emitting a
X-Globus-Client-Infoheaderwhich encodes the SDK version, and can easily be expanded to include other
product name/version combinations. (e.g. compute SDK, globus-cli, etc)
X-Globus-Client-Infois a recent spec we've put together in collaborationwith our frontend dev team and supersedes the use of a custom User-Agent to
encode app information (
app_name). We needed some new art in this space assetting a custom User-Agent is not an appropriate solution for the browser
context.
The
app_namevalue andUser-Agentbehavior is not deprecated at thistime. In future SDK versions, we may choose to deprecate that behavior if we
feel it is in our interest to only provide this data via the new header.
What
Add a new class to implement basic encoding of the header,
GlobusClientInfo. This class is documented and provides the publicinterface for this component. However, it is only exported at the
globus_sdk.transportlayer, as most users will not need to interact withit directly.
Add a private, custom subclass,
DefaultGlobusClientInfo, which seedsproduct=python-sdk,version={globus_sdk.__version__}on init.RequestsTransporthas been enhanced to includeself.globus_clientinfo = DefaultGlobusClientInfo()on init, meaning thatit seeds and stores this information in a publicly accessible attribute.
When
RequestsTransportbuilds headers for a request, it now also includesself.globus_clientinfo.Notes
DefaultGlobusClientInfobehavior could simply be baked into the baseGlobusClientInfo-- input and debate welcome. There are variousequally-valid ways of seeding this information. The goals are to ensure that
this data is set for all SDK calls, that the implementation is easy to
understand, and that it is easy to extend, modify, and even disable.
A clear target use-case here is
globus-cli. The intent here is that theCLI can update such that each client builder adds a call to
A better/easier hooking interface could be added in the future, if one can be
designed based on our experience in that context.
📚 Documentation preview 📚: https://globus-sdk-python--990.org.readthedocs.build/en/990/