Skip to content

Versioned specs #946

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

Versioned specs #946

wants to merge 3 commits into from

Conversation

genematx
Copy link
Contributor

@genematx genematx commented Apr 7, 2025

Checklist

  • Add a Changelog entry
  • Add the ticket number which this PR closes to the comment section

@genematx genematx requested a review from danielballan April 7, 2025 16:38
@danielballan
Copy link
Member

Currently, the registered entrypoint must point to a class implementing the interface of a BaseClient:

tiled/tiled/client/base.py

Lines 114 to 122 in 2d814d5

def __init__(
self,
context: Context,
*,
item,
structure_clients,
structure=None,
include_data_sources=False,
):

It is instantiated like so:

tiled/tiled/client/utils.py

Lines 194 to 200 in 2d814d5

return class_(
context=context,
item=item,
structure_clients=structure_clients,
structure=structure,
include_data_sources=include_data_sources,
)

With the addition in this PR, the target could either be a BaseClient or a function that takes a version and returns a BaseClient:

def f(version):
    if ...:
        return ClientA
    else:
        return ClientB

I thought of two different alternatives for consideration:

  1. Dispatch on the complete spec.
def f(spec):
    ...

I could imagine cases where someone might register the same function f for more than one spec.name. In general I have found it prudent to provide callbacks with a "return address" of what they have been registered on. (See also #919 and caproto/caproto#547.)

  1. Aim for type stability. In main, the regsitered entrypoint always has type BaseClient. If we introduce f, the interface becomes something like Union[BaseClient, Callable[str, BaseClient]] which is a bit more complex to reason about and test. It is possible to satisfy our requirements with one interface, insert spec at the beginning of the signature:
def f(spec, context, * item, structure_clients, structure, include_data_sources):
    if ...:
        return ClientA(context, item=item, structure_clients=structure_clients, ...)
    else:
        return ClientB(context, item=item, structure_clients=structure_clients, ...)

If all BaseClient instances were updated to have this interface (where spec may be None) we could have a type-stable interface for all.


I am open to the solution proposed this in PR---no strong concerns---but thought I would offer these for discussion.

@danielballan
Copy link
Member

This is no longer needed for bluesky-tiled-plugins, so we're going to put this aside. We are open to adding something like this if there is a use case for it, but we won't add it proactively.

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