From fd58941fffbbb79c68186cc20103d3262e1a11b9 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Tue, 4 Jun 2024 14:20:53 -0500 Subject: [PATCH 1/5] Add initial implementation of X-Globus-ClientInfo 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. --- src/globus_sdk/transport/_clientinfo.py | 95 +++++++++++++++++++++++++ src/globus_sdk/transport/requests.py | 7 +- tests/unit/transport/test_clientinfo.py | 41 +++++++++++ 3 files changed, 142 insertions(+), 1 deletion(-) create mode 100644 src/globus_sdk/transport/_clientinfo.py create mode 100644 tests/unit/transport/test_clientinfo.py diff --git a/src/globus_sdk/transport/_clientinfo.py b/src/globus_sdk/transport/_clientinfo.py new file mode 100644 index 000000000..24cd75cfb --- /dev/null +++ b/src/globus_sdk/transport/_clientinfo.py @@ -0,0 +1,95 @@ +""" +This module models a read-write object representation of the +X-Globus-ClientInfo header. + +The spec for X-Globus-ClientInfo is as follows: + +Header Name: X-Globus-Client-Info +Header Value + - A semicolon (;) separated list of client information. + - Client information is a comma-separated list of `=` delimited key-value pairs. + Well-known values for client-information are: + product - A unique identifier of the product. + version - Relevant version information for the product. + - Based on the above, the characters `;,=` should be considered reserved and + should NOT be included in client information values to ensure proper parsing. + +Examples: + X-Globus-ClientInfo: product=python-sdk,version=3.32.1 + X-Globus-ClientInfo: product=python-sdk,version=3.32.1;product=cli,version=4.0.0a1 +""" + +from __future__ import annotations + +import typing as t + +from globus_sdk import exc +from globus_sdk.version import __version__ + +_RESERVED_CHARS = ";,=" + + +class GlobusClientInfo: + """ + An implementation of X-Globus-Client-Info as an object model. + """ + + def __init__(self) -> None: + self.infos: list[str] = [] + + def __bool__(self) -> bool: + """Check if there are any values present.""" + return bool(self.infos) + + def format(self) -> str: + """Format as a header value.""" + return ";".join(self.infos) + + def add(self, value: str | dict[str, str]) -> None: + """ + Add an item to the clientinfo. The item is either already formatted + as a string, or is a dict containing values to format. + + :param value: The element to add to the client-info. If it is a dict, + it may not contain reserved characters in any keys or values. If it is a + string, it cannot contain the ';' separator. + """ + if not isinstance(value, str): + value = ",".join(_format_items(value)) + elif ";" in value: + raise exc.GlobusSDKUsageError( + "GlobusClientInfo.add() cannot be used to add multiple items in " + "an already-joined string. Add items separately instead. " + f"Bad usage: '{value}'" + ) + self.infos.append(value) + + +class DefaultGlobusClientInfo(GlobusClientInfo): + """ + A variant of GlobusClientInfo which always initializes itself to start with + + product=python-sdk,version=... + + using the current package version information. + """ + + def __init__(self) -> None: + super().__init__() + self.add({"product": "python-sdk", "version": __version__}) + + +def _format_items(info: dict[str, str]) -> t.Iterable[str]: + """Format the items in a dict, yielding the contents as an iterable.""" + for key, value in info.items(): + _check_reserved_chars(key, value) + yield f"{key}={value}" + + +def _check_reserved_chars(key: str, value: str) -> None: + """Check a key-value pair to see if it uses reserved chars.""" + if any(c in x for c in _RESERVED_CHARS for x in (key, value)): + raise exc.GlobusSDKUsageError( + "X-Globus-Client-Info reserved characters cannot be used in keys or " + f"values. Bad usage: '{key}: {value}'" + ) diff --git a/src/globus_sdk/transport/requests.py b/src/globus_sdk/transport/requests.py index 0741bf5f2..f0902f977 100644 --- a/src/globus_sdk/transport/requests.py +++ b/src/globus_sdk/transport/requests.py @@ -18,6 +18,7 @@ ) from globus_sdk.version import __version__ +from ._clientinfo import DefaultGlobusClientInfo, GlobusClientInfo from .retry import ( RetryCheck, RetryCheckFlags, @@ -114,6 +115,7 @@ def __init__( self.verify_ssl = config.get_ssl_verify(verify_ssl) self.http_timeout = config.get_http_timeout(http_timeout) self._user_agent = self.BASE_USER_AGENT + self.globus_clientinfo: GlobusClientInfo = DefaultGlobusClientInfo() # retry parameters self.retry_backoff = retry_backoff @@ -135,7 +137,10 @@ def user_agent(self, value: str) -> None: @property def _headers(self) -> dict[str, str]: - return {"Accept": "application/json", "User-Agent": self.user_agent} + headers = {"Accept": "application/json", "User-Agent": self.user_agent} + if self.globus_clientinfo: + headers["X-Globus-ClientInfo"] = self.globus_clientinfo.format() + return headers @contextlib.contextmanager def tune( diff --git a/tests/unit/transport/test_clientinfo.py b/tests/unit/transport/test_clientinfo.py new file mode 100644 index 000000000..eda1db345 --- /dev/null +++ b/tests/unit/transport/test_clientinfo.py @@ -0,0 +1,41 @@ +import pytest + +from globus_sdk.transport._clientinfo import DefaultGlobusClientInfo, GlobusClientInfo + + +def test_clientinfo_bool_after_init(): + # base clientinfo starts empty and should bool false + base = GlobusClientInfo() + assert bool(base) is False + # default clientinfo starts with the SDK version and should bool true + default = DefaultGlobusClientInfo() + assert bool(default) is True + + +@pytest.mark.parametrize( + "value, expect_str", + ( + ("x=y", "x=y"), + ("x=y,omicron=iota", "x=y,omicron=iota"), + ({"x": "y"}, "x=y"), + ({"x": "y", "alpha": "b01"}, "x=y,alpha=b01"), + ), +) +def test_format_of_simple_item(value, expect_str): + info = GlobusClientInfo() + info.add(value) + assert info.format() == expect_str + + +@pytest.mark.parametrize( + "values, expect_str", + ( + (("x=y",), "x=y"), + (("x=y", "alpha=b01,omicron=iota"), "x=y;alpha=b01,omicron=iota"), + ), +) +def test_format_of_multiple_items(values, expect_str): + info = GlobusClientInfo() + for value in values: + info.add(value) + assert info.format() == expect_str From 1aa320805d6ffaf4402b9d2727a82f64ca268f65 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Fri, 21 Jun 2024 12:37:25 -0500 Subject: [PATCH 2/5] Test parsing of X-Globus-Client-Info header - 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 --- tests/unit/transport/test_clientinfo.py | 64 +++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/tests/unit/transport/test_clientinfo.py b/tests/unit/transport/test_clientinfo.py index eda1db345..ff4fad89a 100644 --- a/tests/unit/transport/test_clientinfo.py +++ b/tests/unit/transport/test_clientinfo.py @@ -1,8 +1,54 @@ import pytest +from globus_sdk import __version__ from globus_sdk.transport._clientinfo import DefaultGlobusClientInfo, GlobusClientInfo +def parse_clientinfo(header): + """ + Sample parser. + + Including this in the testsuite not only validates the mechanical implementation of + X-Globus-Client-Info, but also acts as a safety check that we've thought through the + ability of consumers to parse this data. + """ + mappings = {} + for segment in header.split(";"): + segment_dict = {} + + segment = segment.strip() + elements = segment.split(",") + + for element in elements: + if "=" not in element: + raise ValueError( + f"Bad X-Globus-Client-Info element: '{element}' in '{header}'" + ) + key, _, value = element.partition("=") + if "=" in value: + raise ValueError( + f"Bad X-Globus-Client-Info element: '{element}' in '{header}'" + ) + if key in segment_dict: + raise ValueError( + f"Bad X-Globus-Client-Info element: '{element}' in '{header}'" + ) + segment_dict[key] = value + if "product" not in segment_dict: + raise ValueError( + "Bad X-GlobusClientInfo segment missing product: " + f"'{segment}' in '{header}'" + ) + product = segment_dict["product"] + if product in mappings: + raise ValueError( + "Bad X-GlobusClientInfo header repeats product: " + f"'{product}' in '{header}'" + ) + mappings[product] = segment_dict + return mappings + + def test_clientinfo_bool_after_init(): # base clientinfo starts empty and should bool false base = GlobusClientInfo() @@ -39,3 +85,21 @@ def test_format_of_multiple_items(values, expect_str): for value in values: info.add(value) assert info.format() == expect_str + + +def test_clientinfo_parses_as_expected(): + default = DefaultGlobusClientInfo() + default.add("alpha=b01,product=my-cool-tool") + header_str = default.format() + + parsed = parse_clientinfo(header_str) + assert parsed == { + "python-sdk": { + "product": "python-sdk", + "version": __version__, + }, + "my-cool-tool": { + "product": "my-cool-tool", + "alpha": "b01", + }, + } From 31a3c817ba1867c603d2c0bff845ad808e52bf81 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Fri, 21 Jun 2024 13:02:34 -0500 Subject: [PATCH 3/5] Add user documentation for GlobusClientInfo - 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. --- ...40621_130106_sirosen_globus_clientinfo.rst | 7 +++ docs/core/transport.rst | 9 +++ src/globus_sdk/transport/__init__.py | 2 + src/globus_sdk/transport/_clientinfo.py | 56 +++++++++++++------ 4 files changed, 56 insertions(+), 18 deletions(-) create mode 100644 changelog.d/20240621_130106_sirosen_globus_clientinfo.rst diff --git a/changelog.d/20240621_130106_sirosen_globus_clientinfo.rst b/changelog.d/20240621_130106_sirosen_globus_clientinfo.rst new file mode 100644 index 000000000..e3a53572a --- /dev/null +++ b/changelog.d/20240621_130106_sirosen_globus_clientinfo.rst @@ -0,0 +1,7 @@ +Added +~~~~~ + +- Clients will now emit a ``X-Globus-Client-Info`` header which reports the + version of the ``globus-sdk`` which was used to send a request. Users may + customize this header further by modifying the ``globus_clientinfo`` object + attached to the transport object. (:pr:`NUMBER`) diff --git a/docs/core/transport.rst b/docs/core/transport.rst index 742faed31..6b4a951e9 100644 --- a/docs/core/transport.rst +++ b/docs/core/transport.rst @@ -14,6 +14,15 @@ Transport :members: :member-order: bysource +``RequestsTransport`` objects include an attribute, ``globus_clientinfo`` which +provides the ``X-Globus-Client-Info`` header which is sent to Globus services. +It is an instance of ``GlobusClientInfo`` pre-populated with ``globus-sdk`` +information. + +.. autoclass:: globus_sdk.transport.GlobusClientInfo + :members: + :member-order: bysource + Retries ~~~~~~~ diff --git a/src/globus_sdk/transport/__init__.py b/src/globus_sdk/transport/__init__.py index 29ea15039..6d2c77e6a 100644 --- a/src/globus_sdk/transport/__init__.py +++ b/src/globus_sdk/transport/__init__.py @@ -1,3 +1,4 @@ +from ._clientinfo import GlobusClientInfo from .encoders import FormRequestEncoder, JSONRequestEncoder, RequestEncoder from .requests import RequestsTransport from .retry import ( @@ -20,4 +21,5 @@ "RequestEncoder", "JSONRequestEncoder", "FormRequestEncoder", + "GlobusClientInfo", ) diff --git a/src/globus_sdk/transport/_clientinfo.py b/src/globus_sdk/transport/_clientinfo.py index 24cd75cfb..dfaf19295 100644 --- a/src/globus_sdk/transport/_clientinfo.py +++ b/src/globus_sdk/transport/_clientinfo.py @@ -2,21 +2,8 @@ This module models a read-write object representation of the X-Globus-ClientInfo header. -The spec for X-Globus-ClientInfo is as follows: - -Header Name: X-Globus-Client-Info -Header Value - - A semicolon (;) separated list of client information. - - Client information is a comma-separated list of `=` delimited key-value pairs. - Well-known values for client-information are: - product - A unique identifier of the product. - version - Relevant version information for the product. - - Based on the above, the characters `;,=` should be considered reserved and - should NOT be included in client information values to ensure proper parsing. - -Examples: - X-Globus-ClientInfo: product=python-sdk,version=3.32.1 - X-Globus-ClientInfo: product=python-sdk,version=3.32.1;product=cli,version=4.0.0a1 +The spec for X-Globus-ClientInfo is documented, in brief, in the GlobusClientInfo class +docstring. """ from __future__ import annotations @@ -31,8 +18,41 @@ class GlobusClientInfo: """ - An implementation of X-Globus-Client-Info as an object model. - """ + An implementation of X-Globus-Client-Info as an object. This header encodes a + mapping of multiple products to versions and potentially other information. + + Values can be added to a clientinfo object via the ``add()`` method. + + .. rubric:: ``X-Globus-Client-Info`` Specification + + Header Name: ``X-Globus-Client-Info`` + + Header Value: + + - A semicolon (``;``) separated list of client information. + + - Client information is a comma-separated list of ``=`` delimited key-value pairs. + Well-known values for client-information are: + + - ``product``: A unique identifier of the product. + - ``version``: Relevant version information for the product. + + - Based on the above, the characters ``;,=`` should be considered reserved and + should NOT be included in client information values to ensure proper parsing. + + .. rubric:: Example Headers + + .. code-block:: none + + X-Globus-ClientInfo: product=python-sdk,version=3.32.1 + X-Globus-ClientInfo: product=python-sdk,version=3.32.1;product=cli,version=4.0.0a1 + + .. note:: + + The ``GlobusClientInfo`` object is not guaranteed to reject all invalid usages. + For example, ``product`` is required to be unique per header, and users are + expected to enforce this in their usage. + """ # noqa: E501 def __init__(self) -> None: self.infos: list[str] = [] @@ -52,7 +72,7 @@ def add(self, value: str | dict[str, str]) -> None: :param value: The element to add to the client-info. If it is a dict, it may not contain reserved characters in any keys or values. If it is a - string, it cannot contain the ';' separator. + string, it cannot contain the ``;`` separator. """ if not isinstance(value, str): value = ",".join(_format_items(value)) From e83dc05c2575877f8182e90b86b25c1f29d9e46d Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Tue, 25 Jun 2024 16:09:08 -0500 Subject: [PATCH 4/5] Refine GlobusClientInfo per review - 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 --- docs/core/transport.rst | 5 ++-- src/globus_sdk/transport/_clientinfo.py | 29 ++++++++----------- src/globus_sdk/transport/requests.py | 8 +++--- tests/unit/transport/test_clientinfo.py | 37 +++++++++++++++---------- 4 files changed, 39 insertions(+), 40 deletions(-) diff --git a/docs/core/transport.rst b/docs/core/transport.rst index 6b4a951e9..69fe4ec0f 100644 --- a/docs/core/transport.rst +++ b/docs/core/transport.rst @@ -14,10 +14,9 @@ Transport :members: :member-order: bysource -``RequestsTransport`` objects include an attribute, ``globus_clientinfo`` which +``RequestsTransport`` objects include an attribute, ``globus_client_info`` which provides the ``X-Globus-Client-Info`` header which is sent to Globus services. -It is an instance of ``GlobusClientInfo`` pre-populated with ``globus-sdk`` -information. +It is an instance of ``GlobusClientInfo``: .. autoclass:: globus_sdk.transport.GlobusClientInfo :members: diff --git a/src/globus_sdk/transport/_clientinfo.py b/src/globus_sdk/transport/_clientinfo.py index dfaf19295..1a7cbdbda 100644 --- a/src/globus_sdk/transport/_clientinfo.py +++ b/src/globus_sdk/transport/_clientinfo.py @@ -1,8 +1,8 @@ """ This module models a read-write object representation of the -X-Globus-ClientInfo header. +X-Globus-Client-Info header. -The spec for X-Globus-ClientInfo is documented, in brief, in the GlobusClientInfo class +The spec for X-Globus-Client-Info is documented, in brief, in the GlobusClientInfo class docstring. """ @@ -23,6 +23,12 @@ class GlobusClientInfo: Values can be added to a clientinfo object via the ``add()`` method. + The object always initializes itself to start with + + product=python-sdk,version=... + + using the current package version information. + .. rubric:: ``X-Globus-Client-Info`` Specification Header Name: ``X-Globus-Client-Info`` @@ -44,8 +50,8 @@ class GlobusClientInfo: .. code-block:: none - X-Globus-ClientInfo: product=python-sdk,version=3.32.1 - X-Globus-ClientInfo: product=python-sdk,version=3.32.1;product=cli,version=4.0.0a1 + X-Globus-Client-Info: product=python-sdk,version=3.32.1 + X-Globus-Client-Info: product=python-sdk,version=3.32.1;product=cli,version=4.0.0a1 .. note:: @@ -56,6 +62,7 @@ class GlobusClientInfo: def __init__(self) -> None: self.infos: list[str] = [] + self.add({"product": "python-sdk", "version": __version__}) def __bool__(self) -> bool: """Check if there are any values present.""" @@ -85,20 +92,6 @@ def add(self, value: str | dict[str, str]) -> None: self.infos.append(value) -class DefaultGlobusClientInfo(GlobusClientInfo): - """ - A variant of GlobusClientInfo which always initializes itself to start with - - product=python-sdk,version=... - - using the current package version information. - """ - - def __init__(self) -> None: - super().__init__() - self.add({"product": "python-sdk", "version": __version__}) - - def _format_items(info: dict[str, str]) -> t.Iterable[str]: """Format the items in a dict, yielding the contents as an iterable.""" for key, value in info.items(): diff --git a/src/globus_sdk/transport/requests.py b/src/globus_sdk/transport/requests.py index f0902f977..819c2a7d3 100644 --- a/src/globus_sdk/transport/requests.py +++ b/src/globus_sdk/transport/requests.py @@ -18,7 +18,7 @@ ) from globus_sdk.version import __version__ -from ._clientinfo import DefaultGlobusClientInfo, GlobusClientInfo +from ._clientinfo import GlobusClientInfo from .retry import ( RetryCheck, RetryCheckFlags, @@ -115,7 +115,7 @@ def __init__( self.verify_ssl = config.get_ssl_verify(verify_ssl) self.http_timeout = config.get_http_timeout(http_timeout) self._user_agent = self.BASE_USER_AGENT - self.globus_clientinfo: GlobusClientInfo = DefaultGlobusClientInfo() + self.globus_client_info: GlobusClientInfo = GlobusClientInfo() # retry parameters self.retry_backoff = retry_backoff @@ -138,8 +138,8 @@ def user_agent(self, value: str) -> None: @property def _headers(self) -> dict[str, str]: headers = {"Accept": "application/json", "User-Agent": self.user_agent} - if self.globus_clientinfo: - headers["X-Globus-ClientInfo"] = self.globus_clientinfo.format() + if self.globus_client_info: + headers["X-Globus-Client-Info"] = self.globus_client_info.format() return headers @contextlib.contextmanager diff --git a/tests/unit/transport/test_clientinfo.py b/tests/unit/transport/test_clientinfo.py index ff4fad89a..77f825a22 100644 --- a/tests/unit/transport/test_clientinfo.py +++ b/tests/unit/transport/test_clientinfo.py @@ -1,7 +1,14 @@ import pytest from globus_sdk import __version__ -from globus_sdk.transport._clientinfo import DefaultGlobusClientInfo, GlobusClientInfo +from globus_sdk.transport._clientinfo import GlobusClientInfo + + +def make_empty_clientinfo(): + # create a clientinfo with no contents, as a starting point for tests + obj = GlobusClientInfo() + obj.infos = [] + return obj def parse_clientinfo(header): @@ -36,26 +43,26 @@ def parse_clientinfo(header): segment_dict[key] = value if "product" not in segment_dict: raise ValueError( - "Bad X-GlobusClientInfo segment missing product: " + "Bad X-Globus-Client-Info segment missing product: " f"'{segment}' in '{header}'" ) product = segment_dict["product"] if product in mappings: raise ValueError( - "Bad X-GlobusClientInfo header repeats product: " + "Bad X-Globus-Client-Info header repeats product: " f"'{product}' in '{header}'" ) mappings[product] = segment_dict return mappings -def test_clientinfo_bool_after_init(): - # base clientinfo starts empty and should bool false - base = GlobusClientInfo() - assert bool(base) is False - # default clientinfo starts with the SDK version and should bool true - default = DefaultGlobusClientInfo() - assert bool(default) is True +def test_clientinfo_bool(): + # base clientinfo starts with the SDK version and should bool true + info = GlobusClientInfo() + assert bool(info) is True + # but we can clear it and it will bool False + info.infos = [] + assert bool(info) is False @pytest.mark.parametrize( @@ -68,7 +75,7 @@ def test_clientinfo_bool_after_init(): ), ) def test_format_of_simple_item(value, expect_str): - info = GlobusClientInfo() + info = make_empty_clientinfo() info.add(value) assert info.format() == expect_str @@ -81,16 +88,16 @@ def test_format_of_simple_item(value, expect_str): ), ) def test_format_of_multiple_items(values, expect_str): - info = GlobusClientInfo() + info = make_empty_clientinfo() for value in values: info.add(value) assert info.format() == expect_str def test_clientinfo_parses_as_expected(): - default = DefaultGlobusClientInfo() - default.add("alpha=b01,product=my-cool-tool") - header_str = default.format() + info = GlobusClientInfo() + info.add("alpha=b01,product=my-cool-tool") + header_str = info.format() parsed = parse_clientinfo(header_str) assert parsed == { From 41c2c78a7a7cc544f5ed64f2e7c829dc2ab18e4c Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Tue, 25 Jun 2024 16:21:10 -0500 Subject: [PATCH 5/5] Add functional tests for X-Globus-Client-Info --- .../base_client/test_default_headers.py | 34 +++++++++++++++++++ tests/unit/transport/test_clientinfo.py | 2 +- 2 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 tests/functional/base_client/test_default_headers.py diff --git a/tests/functional/base_client/test_default_headers.py b/tests/functional/base_client/test_default_headers.py new file mode 100644 index 000000000..5bfce98be --- /dev/null +++ b/tests/functional/base_client/test_default_headers.py @@ -0,0 +1,34 @@ +from globus_sdk import __version__ +from globus_sdk._testing import RegisteredResponse, get_last_request + + +def test_clientinfo_header_default(client): + RegisteredResponse( + path="https://foo.api.globus.org/bar", + json={"foo": "bar"}, + ).add() + res = client.request("GET", "/bar") + assert res.http_status == 200 + + req = get_last_request() + assert "X-Globus-Client-Info" in req.headers + assert ( + req.headers["X-Globus-Client-Info"] + == f"product=python-sdk,version={__version__}" + ) + + +def test_clientinfo_header_can_be_supressed(client): + RegisteredResponse( + path="https://foo.api.globus.org/bar", + json={"foo": "bar"}, + ).add() + + # clear the X-Globus-Client-Info header + client.transport.globus_client_info.infos = [] + + res = client.request("GET", "/bar") + assert res.http_status == 200 + + req = get_last_request() + assert "X-Globus-Client-Info" not in req.headers diff --git a/tests/unit/transport/test_clientinfo.py b/tests/unit/transport/test_clientinfo.py index 77f825a22..43c9567b9 100644 --- a/tests/unit/transport/test_clientinfo.py +++ b/tests/unit/transport/test_clientinfo.py @@ -1,7 +1,7 @@ import pytest from globus_sdk import __version__ -from globus_sdk.transport._clientinfo import GlobusClientInfo +from globus_sdk.transport import GlobusClientInfo def make_empty_clientinfo():