Skip to content
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

[Feature Request] Standardize method for listing workflow queries/signals (and maybe other things like registered activities/workflows and other metadata) #51

Open
2 of 6 tasks
bergundy opened this issue Oct 25, 2021 · 13 comments
Labels
enhancement New feature or request

Comments

@bergundy
Copy link
Member

bergundy commented Oct 25, 2021

Up until now we've used a hack to get the list of registered queries from a workflow execution.
The hack is to send a query to a workflow, which responds with something like "query not found, list of registered queries...", parse the response and show in the UI.

A standard way to support listing workflow queries would be to have a "listQueries" (name TBD) built-in query provided by the Workflow runtime.

We'd have to keep the hack around to support SDKs that still do not support the listQueries built-in query (all at the time of this writing).

UPDATE: Here is the roadmap:

@cretz
Copy link
Member

cretz commented Nov 1, 2021

There are other things that are only registered at the SDK level, e.g. signal handlers. I wonder if we want a more generic "get SDK workflow details" query that also shows signal handlers and potentially any other SDK-side-only metadata. Whatever is decided, of course it needs to be well specified since all SDKs need to support it in the exact same way.

@bergundy
Copy link
Member Author

bergundy commented Nov 1, 2021

That's a good suggestion @cretz, let's do that.

@Sushisource
Copy link
Member

We should define the message in the API proto repo

@bergundy bergundy changed the title [Feature Request] Standardize method for listing workflow queries [Feature Request] Standardize method for listing workflow query and signal handler Jan 13, 2022
@bergundy bergundy changed the title [Feature Request] Standardize method for listing workflow query and signal handler [Feature Request] Standardize method for listing workflow query and signal handlers Jan 13, 2022
@Spikhalskiy Spikhalskiy transferred this issue from temporalio/web May 4, 2022
@feedmeapples
Copy link

feedmeapples commented Sep 6, 2022

is it possible in theory to also provide input data type definitions? for both Queries and Signals in case we want to dynamically construct the UI

@cretz
Copy link
Member

cretz commented Sep 6, 2022

@feedmeapples - Can discuss in separate issue/discussion as that's much more complex (and due to signals persisting in history even on bad input poses some issues alleviated by the upcoming synchronous update feature).

@cretz
Copy link
Member

cretz commented Mar 27, 2023

Implementation Thoughts

This should be represented as proto which every language and the UI can use. Granted the format will be JSON-based proto
not binary.

Here would be my idea (expressed in TS terms, but applicable to all SDKs):

// This can be per workflow run or per workflow type
message WorkflowMetadata {
  WorkflowDefinition definition = 1;

  // TODO(cretz): Future possibilities
  // * Stack information
  // * Worker metadata sans workflow list
}

message WorkflowDefinition {
  // If empty, this is a "dynamic" form of the workflow
  string type = 1;
  // Optional
  string description = 2;

  // When fetching at a worker level these are obviously fixed to the workflow
  // definition which means that languages like Go need to provide a way to
  // allow developers to define these things ahead of time if they want (unlike
  // today which is runtime). When fetching at a workflow level, these may be
  // specific to a run since handlers can be added inside of workflows.
  //
  // We can break these out to separate message types for each definition type
  // if/when there are fields specific to each.
  repeated InteractionDefinition signal_definitions = 3;
  repeated InteractionDefinition query_definitions = 4;
  repeated InteractionDefinition update_definitions = 5;
}

message InteractionDefinition {
  // If empty, this is a "dynamic" form of the interaction
  string name = 1;

  // Optional. While we can support type hinting information for callers in
  // more well-typed fields in the future, it gets complicated quickly.
  // Therefore, all information a caller may need should currently be in English
  // in this field (even if it's generated by the SDK).
  string description = 2;
}

message ActivityDefinition {
  // If empty, this is a "dynamic" form of the activity
  string type = 1;
  // Optional
  string description = 2;
}

message WorkerMetadata {
  repeated WorkflowDefinition workflow_definitions = 1;
  repeated ActivityDefinition activity_definitions = 2;

  // TODO(cretz): There are so many options here to add things such as:
  // * Registration/config options
  // * Cache info
  // * Identity information we usually have for workers
  // * SDK language and version
  // * General process/metric info (but don't overdo this)
}

Here are the calls one can make:

  • "get workflow metadata" - Issued to workflow as a query
  • "get worker metadata" - Issued to the worker via a new concept I'm calling "worker updates". Implementation thoughts:
    • New UpdateWorker top-level RPC call that accepts a task queue and optional worker identity
    • The repeated temporal.api.protocol.v1.Message messages needs to also be added to PollActivityTaskQueueResponse
    • Upon worker update, the next poll of PollWorkflowTaskQueueResponse or PollActivityTaskQueueResponse will be the
      worker update which is represented as:
      • Either a new "task queue kind" or special task queue name
      • ^ Once that is seen, no task response is expected by server and anything else about the poll response except the
        worker update is discarded (no need to piggyback on existing tasks I don't think, it adds confusion)
      • Worker update request is inside the protocol message
    • Workers response to worker updates via a new RespondWorkerUpdate top-level call which takes arbitrary protocol
      messages/identifiers
    • Technically we can expose worker update handlers, but there is not much value today
      • Consideration was given to just using activities or workflows as "worker update", but we can't guarantee what is
        being polled on

For discussion:

  1. Is "metadata" a good word? Even though we are only talking about "definitions" here, I want to encompass the idea
    of runtime information that is not necessarily definition based
  2. Maybe we should break "description" into "summary" and "description" to encourage one-liner "summary" for use in UI listing and other places
  3. Is there value yet in asking an activity for runtime "metadata" per activity run (obv as a request on a heartbeat
    response)?
  4. How does each language represent its "description" information. Some languages like Python can do neat things like
    grab the __doc__ and also add type hint information, but most other languages do not have docs at runtime so a
    separate field at the registration/declaration/definition/decoration/annotation/attribute site would be required
  5. How best to allow Go to predefine its interactions?
  6. Be sure to ask UI team if this works for them and/or if they have any suggestions
  7. For an initial version of this, we can just support the "get workflow metadata" operation as a query and ignore all that "worker update" business, but we need to keep it in mind so we design our metadata approach where it could be obtained at a worker level

@cretz cretz changed the title [Feature Request] Standardize method for listing workflow query and signal handlers [Feature Request] Standardize method for listing workflow queries/signals (and maybe other things like registered activities/workflows and other metadta) Jun 15, 2023
@cretz cretz changed the title [Feature Request] Standardize method for listing workflow queries/signals (and maybe other things like registered activities/workflows and other metadta) [Feature Request] Standardize method for listing workflow queries/signals (and maybe other things like registered activities/workflows and other metadata) Jun 15, 2023
@Alex-Tideman
Copy link

I would love to get this listQueries feature in all the SDKs. We are envisioning building custom UIs based on query names and it would be a lot nicer to do this without the current hack. Happy to work with whoever on this from the SDK side.

@antlai-temporal
Copy link
Contributor

  // If empty, this is a "dynamic" form of the workflow

@cretz I wonder if we need to report dynamic workflow/interactions at all. In particular, if they don't have a description either, there is no proper way to identify them...

@cretz
Copy link
Member

cretz commented Dec 6, 2023

I wonder if we need to report dynamic workflow/interactions at all. In particular, if they don't have a description either, there is no proper way to identify them...

This is about providing information to the caller about the shape of what is defined on the workflow. The caller may want to know that any query could be accepted because dynamic query is enabled. This can affect things like the UI where they may make it clear to a user that any arbitrary string query name would be accepted if they knew that dynamic queries were supported.

Btw, if you are tackling this now, I would ignore anything about "worker queries" in this issue, that can be done somewhere else.

@antlai-temporal
Copy link
Contributor

antlai-temporal commented Dec 7, 2023

. The caller may want to know that any query could be accepted because dynamic query is enabled
Do we have the same concept for updates and signals, or just only for queries? The examples I'm seeing define a generic signal first, and add extra info in the payload, so name will not be missing... Mistery solved, dynamic handlers are not supported in TS, only Java/Python/.Net, and go uses GetUnhandledSignalNames for signals. For update, there is no GetUnhandledUpdateNames in go (I'm adding an issue), Java seems to support it, not sure about .Net/Python...

@cretz
Copy link
Member

cretz commented Dec 7, 2023

This issue is known and #201 exists to bring parity here.

Signals in Go for this feature request are going to be a bit interesting. My suggestion: there is no such thing as a signal definition in Go so we should never populate that repeated field in metadata. However, we can create helper API in Go like workflow.DefineSignalChannel(workflow.SignalDefinition{Name: "my-signal", Description: "whatever"}) and people can call that to populate this info but it won't relate to their actual channel use. It would be a bad idea IMO to populate Go signal definitions in this metadata just based on whether a signal channel was ever asked for and similarly bad if it included any signal name that had been seen.

We're going to need to design how to provide "descriptions" to these interactions in every language anyways. In Go, SetUpdateHandlerWithOptions's options can have it, but you'd need a new SetQueryHandlerWithOptions for queries. In Java, Python, and .NET you can use a field on the annotation, decorator, and attribute respectively though you need to still accept them on runtime-added forms. In TS these are likely just options on the define calls.

I can provide clarification here if unclear.

@Quinn-With-Two-Ns
Copy link
Contributor

Quinn-With-Two-Ns commented Jan 29, 2024

This needs to be done in every SDK other than TypeScript

@Sushisource
Copy link
Member

Sushisource commented May 24, 2024

Note from UI sync: Good idea to have an "is visible in UI" flag for interactions

Other possible future options:

  • Disable cancel
  • possibility to customize input ui / forms etc
  • Some kind of markup for possible interactions / form in the UI for the workflow

@mjameswh mjameswh removed their assignment May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

9 participants