From 4d339c9d448e0b3a82a94afdc49b7d708110b392 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Mon, 16 Oct 2023 10:58:41 -0500 Subject: [PATCH 1/4] Add 'limit' and 'offset' params to 'ls' These two parameters to the TransferClient.operation_ls call were missing. The Transfer API documentation has notes on pagination with limit+offset included in the documentation for `limit`. Rather than doing this, pull that same basic content into a `.. note::` directive. --- .../20231016_114115_sirosen_add_ls_params.rst | 5 +++++ src/globus_sdk/services/transfer/client.py | 22 +++++++++++++++++++ 2 files changed, 27 insertions(+) create mode 100644 changelog.d/20231016_114115_sirosen_add_ls_params.rst diff --git a/changelog.d/20231016_114115_sirosen_add_ls_params.rst b/changelog.d/20231016_114115_sirosen_add_ls_params.rst new file mode 100644 index 000000000..f6ca568a7 --- /dev/null +++ b/changelog.d/20231016_114115_sirosen_add_ls_params.rst @@ -0,0 +1,5 @@ +Added +~~~~~ + +- ``TransferClient.operation_ls`` now supports the ``limit`` and ``offset`` + parameters (:pr:`NUMBER`) diff --git a/src/globus_sdk/services/transfer/client.py b/src/globus_sdk/services/transfer/client.py index af923058a..6168ba791 100644 --- a/src/globus_sdk/services/transfer/client.py +++ b/src/globus_sdk/services/transfer/client.py @@ -1210,6 +1210,8 @@ def operation_ls( *, show_hidden: bool | None = None, orderby: str | list[str] | None = None, + limit: int | None = None, + offset: int | None = None, # note: filter is a soft keyword in python, so using this name is okay # pylint: disable=redefined-builtin filter: ( @@ -1226,6 +1228,11 @@ def operation_ls( :param show_hidden: Show hidden files (names beginning in dot). Defaults to true. :type show_hidden: bool, optional + :param limit: Limit the number of results returned. Defaults to 100,000 , + which is also the maximum. + :type limit: int, optional + :param offset: Offset into the result list, which can be used to page results. + :type offset: int, optional :param orderby: One or more order-by options. Each option is either a field name or a field name followed by a space and 'ASC' or 'DESC' for ascending or descending. @@ -1243,6 +1250,17 @@ def operation_ls( :param query_params: Additional passthrough query parameters :type query_params: dict, optional + .. note:: + + Pagination is not supported by the GridFTP protocol, and therefore + limit+offset pagination will result in the Transfer service repeatedly + fetching an entire directory listing from the server and filtering it before + retuning it to the client. + + Such usage may still be more efficient than asking for a very large directory + listing, for latency-sensitive applications, as it reduces the size of the payload + passed between the Transfer service and the client. + .. tab-set:: .. tab-item:: Example Usage @@ -1289,6 +1307,10 @@ def operation_ls( query_params["path"] = path if show_hidden is not None: query_params["show_hidden"] = 1 if show_hidden else 0 + if limit is not None: + query_params["limit"] = limit + if offset is not None: + query_params["offset"] = offset if orderby is not None: if isinstance(orderby, str): query_params["orderby"] = orderby From f4340cf459ab09b3a53959ab4c9d70482d4284f3 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Mon, 16 Oct 2023 11:39:09 -0500 Subject: [PATCH 2/4] Convert operation_ls tests to newer style - Remove separate fixture file (generate it inline as part of the test file) and move it to `globus_sdk._testing` style - Move the method test to a dedicated file with parametrized tests --- .../fixture_data/operation_ls_goep1.json | 79 ---------------- .../services/transfer/test_operation_ls.py | 90 +++++++++++++++++++ .../services/transfer/test_simple.py | 54 ----------- 3 files changed, 90 insertions(+), 133 deletions(-) delete mode 100644 tests/functional/services/transfer/fixture_data/operation_ls_goep1.json create mode 100644 tests/functional/services/transfer/test_operation_ls.py diff --git a/tests/functional/services/transfer/fixture_data/operation_ls_goep1.json b/tests/functional/services/transfer/fixture_data/operation_ls_goep1.json deleted file mode 100644 index c9ec06dd6..000000000 --- a/tests/functional/services/transfer/fixture_data/operation_ls_goep1.json +++ /dev/null @@ -1,79 +0,0 @@ -{ - "DATA": [ - { - "DATA_TYPE": "file", - "group": "tutorial", - "last_modified": "2021-02-26 18:35:02+00:00", - "link_group": null, - "link_last_modified": null, - "link_size": null, - "link_target": null, - "link_user": null, - "name": "foo", - "permissions": "0755", - "size": 4096, - "type": "dir", - "user": "sirosen" - }, - { - "DATA_TYPE": "file", - "group": "tutorial", - "last_modified": "2021-05-13 15:06:10+00:00", - "link_group": null, - "link_last_modified": null, - "link_size": null, - "link_target": null, - "link_user": null, - "name": "tempdir1", - "permissions": "0755", - "size": 4096, - "type": "dir", - "user": "sirosen" - }, - { - "DATA_TYPE": "file", - "group": "tutorial", - "last_modified": "2018-04-04 18:30:26+00:00", - "link_group": null, - "link_last_modified": null, - "link_size": null, - "link_target": null, - "link_user": null, - "name": ".bash_logout", - "permissions": "0644", - "size": 220, - "type": "file", - "user": "sirosen" - }, - { - "DATA_TYPE": "file", - "group": "tutorial", - "last_modified": "2018-04-04 18:30:26+00:00", - "link_group": null, - "link_last_modified": null, - "link_size": null, - "link_target": null, - "link_user": null, - "name": ".bashrc", - "permissions": "0644", - "size": 3771, - "type": "file", - "user": "sirosen" - }, - { - "DATA_TYPE": "file", - "group": "tutorial", - "last_modified": "2018-04-04 18:30:26+00:00", - "link_group": null, - "link_last_modified": null, - "link_size": null, - "link_target": null, - "link_user": null, - "name": ".profile", - "permissions": "0644", - "size": 807, - "type": "file", - "user": "sirosen" - } - ] -} diff --git a/tests/functional/services/transfer/test_operation_ls.py b/tests/functional/services/transfer/test_operation_ls.py new file mode 100644 index 000000000..14ba9cf69 --- /dev/null +++ b/tests/functional/services/transfer/test_operation_ls.py @@ -0,0 +1,90 @@ +import urllib.parse + +import pytest + +from globus_sdk._testing import RegisteredResponse, get_last_request, load_response +from tests.common import GO_EP1_ID + + +def _mk_item(*, name, typ, size=0): + return { + "DATA_TYPE": "file", + "group": "tutorial", + "last_modified": "2018-04-04 18:30:26+00:00", + "link_group": None, + "link_last_modified": None, + "link_size": None, + "link_target": None, + "link_user": None, + "name": name, + "permissions": "0755" if typ == "dir" else "0644", + "size": 4096 if typ == "dir" else size, + "type": typ, + "user": "snork", + } + + +def _mk_ls_data(): + return { + "DATA": [ + _mk_item(name="foo", typ="dir"), + _mk_item(name="tempdir1", typ="dir"), + _mk_item(name=".bashrc", typ="file", size=3771), + _mk_item(name=".profile", typ="file", size=807), + ] + } + + +@pytest.fixture(autouse=True) +def _setup_ls_response(): + load_response( + RegisteredResponse( + service="transfer", + path=f"/operation/endpoint/{GO_EP1_ID}/ls", + json=_mk_ls_data(), + ), + ) + + +def test_operation_ls(client): + ls_path = f"https://transfer.api.globus.org/v0.10/operation/endpoint/{GO_EP1_ID}/ls" + + # load the tutorial endpoint ls doc + ls_doc = client.operation_ls(GO_EP1_ID) + + # check that the result is an iterable of file and dir dict objects + for x in ls_doc: + assert "DATA_TYPE" in x + assert x["DATA_TYPE"] in ("file", "dir") + + req = get_last_request() + assert req.url == ls_path + + +@pytest.mark.parametrize( + "kwargs, expected_qs", + [ + # orderby with a single str + ({"orderby": "name"}, {"orderby": ["name"]}), + # orderby with a multiple strs + ( + {"orderby": ["size DESC", "name", "type"]}, + {"orderby": ["size DESC,name,type"]}, + ), + # orderby + filter + ( + {"orderby": "name", "filter": "name:~*.png"}, + {"orderby": ["name"], "filter": ["name:~*.png"]}, + ), + # local_user + ( + {"local_user": "my-user"}, + {"local_user": ["my-user"]}, + ), + ], +) +def test_operation_ls_params(client, kwargs, expected_qs): + client.operation_ls(GO_EP1_ID, **kwargs) + req = get_last_request() + parsed_qs = urllib.parse.parse_qs(urllib.parse.urlparse(req.url).query) + assert parsed_qs == expected_qs diff --git a/tests/functional/services/transfer/test_simple.py b/tests/functional/services/transfer/test_simple.py index 195cac46a..630cdbab4 100644 --- a/tests/functional/services/transfer/test_simple.py +++ b/tests/functional/services/transfer/test_simple.py @@ -100,60 +100,6 @@ def test_create_endpoint_invalid_activation_servers(client): assert "either MyProxy or OAuth, not both" in str(excinfo.value) -def test_operation_ls(client): - """ - Does an `ls` on go#ep1, validate results, and check that the request parameters were - formatted and sent correctly. - """ - # register get_endpoint mock data - register_api_route_fixture_file( - "transfer", f"/operation/endpoint/{GO_EP1_ID}/ls", "operation_ls_goep1.json" - ) - ls_path = f"https://transfer.api.globus.org/v0.10/operation/endpoint/{GO_EP1_ID}/ls" - - # load the tutorial endpoint ls doc - ls_doc = client.operation_ls(GO_EP1_ID) - - # check that the result is an iterable of file and dir dict objects - count = 0 - for x in ls_doc: - assert "DATA_TYPE" in x - assert x["DATA_TYPE"] in ("file", "dir") - count += 1 - # not exact, just make sure the fixture wasn't empty - assert count > 3 - - req = get_last_request() - assert req.url == ls_path - - # don't change the registered response - # the resulting data might be "wrong", but we're just checking request formatting - - # orderby with a single str - client.operation_ls(GO_EP1_ID, orderby="name") - req = get_last_request() - parsed_qs = urllib.parse.parse_qs(urllib.parse.urlparse(req.url).query) - assert parsed_qs == {"orderby": ["name"]} - - # orderby multiple strs - client.operation_ls(GO_EP1_ID, orderby=["size DESC", "name", "type"]) - req = get_last_request() - parsed_qs = urllib.parse.parse_qs(urllib.parse.urlparse(req.url).query) - assert parsed_qs == {"orderby": ["size DESC,name,type"]} - - # orderby + filter - client.operation_ls(GO_EP1_ID, orderby="name", filter="name:~*.png") - req = get_last_request() - parsed_qs = urllib.parse.parse_qs(urllib.parse.urlparse(req.url).query) - assert parsed_qs == {"orderby": ["name"], "filter": ["name:~*.png"]} - - # local_user - client.operation_ls(GO_EP1_ID, local_user="my-user") - req = get_last_request() - parsed_qs = urllib.parse.parse_qs(urllib.parse.urlparse(req.url).query) - assert parsed_qs == {"local_user": ["my-user"]} - - def test_autoactivation(client): """ Do `autoactivate` on go#ep1, validate results, and check that `if_expires_in` can be From 7397af6f89c78d000389418fff4f5521a9ce6f4c Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Mon, 16 Oct 2023 11:41:02 -0500 Subject: [PATCH 3/4] Add limit+offset to ls param tests --- tests/functional/services/transfer/test_operation_ls.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/functional/services/transfer/test_operation_ls.py b/tests/functional/services/transfer/test_operation_ls.py index 14ba9cf69..f4c8438d6 100644 --- a/tests/functional/services/transfer/test_operation_ls.py +++ b/tests/functional/services/transfer/test_operation_ls.py @@ -81,6 +81,11 @@ def test_operation_ls(client): {"local_user": "my-user"}, {"local_user": ["my-user"]}, ), + # limit+offset + ( + {"limit": 10, "offset": 5}, + {"limit": ["10"], "offset": ["5"]}, + ), ], ) def test_operation_ls_params(client, kwargs, expected_qs): From 4b3f53b06eb76a999efc3d37c0d7b23176aa71c6 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Mon, 16 Oct 2023 14:20:31 -0500 Subject: [PATCH 4/4] Update src/globus_sdk/services/transfer/client.py Co-authored-by: Kurt McKee --- src/globus_sdk/services/transfer/client.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/globus_sdk/services/transfer/client.py b/src/globus_sdk/services/transfer/client.py index 6168ba791..67517c121 100644 --- a/src/globus_sdk/services/transfer/client.py +++ b/src/globus_sdk/services/transfer/client.py @@ -1255,11 +1255,11 @@ def operation_ls( Pagination is not supported by the GridFTP protocol, and therefore limit+offset pagination will result in the Transfer service repeatedly fetching an entire directory listing from the server and filtering it before - retuning it to the client. + returning it to the client. - Such usage may still be more efficient than asking for a very large directory - listing, for latency-sensitive applications, as it reduces the size of the payload - passed between the Transfer service and the client. + For latency-sensitive applications, such usage may still be more efficient + than asking for a very large directory listing as it reduces the size of the + payload passed between the Transfer service and the client. .. tab-set::