Skip to content

Add experimental GCSCollectionClient#1375

Open
sirosen wants to merge 3 commits intoglobus:mainfrom
sirosen:gcs-collection-client
Open

Add experimental GCSCollectionClient#1375
sirosen wants to merge 3 commits intoglobus:mainfrom
sirosen:gcs-collection-client

Conversation

@sirosen
Copy link
Member

@sirosen sirosen commented Mar 7, 2026

The new client definition is minimal, providing no methods. It only maps
collection ID + base URL to existing SDK methods, and defines the scopes
appropriately for the client.

Based on the solution for scope as a class-attribute of clients used in
the SpecificFlowClient, a stub is defined for the scopes of this client
class which raises appropriate attribute errors on access.
It thereby makes itself type-safe in contexts where it is used correctly.
(Adjusting such that scope based access at the class level is a type
error for this class and SpecificFlowClient is considered out of scope
for this change.)

Unit tests for the scope behaviors + a new (small) doc page are included.

The new client definition is minimal, providing no methods. It only maps
collection ID + base URL to existing SDK methods, and defines the scopes
appropriately for the client.

Based on the solution for `scope` as a class-attribute of clients used in
the `SpecificFlowClient`, a stub is defined for the scopes of this client
class which raises appropriate attribute errors on access.
It thereby makes itself type-safe in contexts where it is used correctly.
(Adjusting such that scope based access at the class level is a type
error for this class and `SpecificFlowClient` is considered out of scope
for this change.)

Unit tests for the scope behaviors + a new (small) doc page are included.
Comment on lines +59 to +60
collection_id: str | uuid.UUID,
base_url: str,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the relationship between these two parameters one where base url can be discovered consistently from transfer collection search -> endpoint id -> endpoint info route?

But that logic was too complex for us to put into a client constructor?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so. But there may even be cases (I'm not certain) where a user cannot view the collection in Transfer but can interact with it directly for upload/download.

I think we could build an alternate constructor or factory that uses a TransferClient in the future, but I'd want to discuss it with the Transfer/GCS devs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I suppose I'm fine leaving that undecided since this is going into experimental, but I will want to make sure we revisit before pulling into the main repo.


Separate question: Could we use a different term from base_url (or at least update the documentation on it to be more collection domain specific).

In the GCSClient we use:

:param gcs_address: The FQDN (DNS name) or HTTPS URL for the GCS Manager API.

The person constructing this client will likely a few different urls to choose from when creating this class. Ideally we would answer that question by the parameter name, but if not a clear explanation in the docstring would go a long way.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I suppose I'm fine leaving that undecided since this is going into experimental, but I will want to make sure we revisit before pulling into the main repo.

Just to be clear -- I don't think this discussion blocks us so maybe not a big deal -- I don't consider this something "undecided".

This client should not require that you use TransferClient in order to instantiate it. I'm fine with adding a feature (in this class or outside of it) which does precisely that, but the __init__ for this class should not change. Changing this definition in order to support such a behavior would be a strong warning sign that we've accidentally broken the orthogonality of components.

We'll end up revisiting, I'm sure, when it's time to promote this out of experimental, so we can leave the debate for later. 😁


Regarding the parameter name...

In the Collection and Endpoint doucments documentation, the relevant field is named https_server.
To me, that feels just slightly wrong as a name in the SDK context.

The gcs_address field most closely matches gcs_manager_url, so we're already a little mismatched.

I'm going to push a change which calls this collection_address, and documents it as "as might be retrieved from the https_server field in Globus Transfer".

sirosen and others added 2 commits March 10, 2026 14:28
Co-authored-by: derek-globus <113056046+derek-globus@users.noreply.github.com>
Use `collection_address` to match `GCSClient`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants