Describe task queue #656
Describe task queue #656
Conversation
| rpc_metadata: Mapping[str, str] = {}, | ||
| rpc_timeout: Optional[timedelta] = None, | ||
| ) -> TaskQueueDescription: | ||
| """Describe task queue. |
There was a problem hiding this comment.
nit: This comment is redundant. Can you give a flavor for the type of information I should expect as a response and why I would use each one?
At this point I assumed I would get some info if I check neither and was wondering what it was. Wasn't until I looked at the code that I realized it wouldn't work.
There was a problem hiding this comment.
👍 I will clarify that this returns poller and stats information and that currently you need to provide at least one of the booleans.
| report_pollers: Include list of pollers for requested task queue types. | ||
| report_stats: Include task queue stats for requested task queue types. |
There was a problem hiding this comment.
Are these toggles here because it could degrade server performance to check them? Is there any kind of throughput limitation people should be aware of? (Otherwise, why are they necessary?)
There was a problem hiding this comment.
This is how the API was designed server side, but I would guess that they made these opt-in for performance reasons, yes. This is consistent with the Go SDK API that exists today already.
| if not report_pollers and not report_stats: | ||
| raise ValueError( | ||
| "At least one of report_pollers or report_stats must be True" | ||
| ) |
There was a problem hiding this comment.
nit: Oh I see, so one is required. I assumed not since they were both optional, and there was a default set of info.
If you want, you could avoid this friction, and runtime errors, by having a required enum field for either/both.
There was a problem hiding this comment.
An enum is a bit rough here because there are more coming (e.g. "report reachability"). We designed this as simple booleans to match Go SDK.
| @@ -1108,6 +1108,46 @@ async def get_worker_task_reachability( | |||
| ) | |||
| ) | |||
|
|
|||
| async def describe_task_queue( | |||
There was a problem hiding this comment.
Have we considered a top level scaling/tuning API instead that takes task queue types as a filter? That might be more discoverable for somebody perusing the CLI/API command surface, who knows they want to do scaling but doesn't think of that when they see task queue.
It would also allow us to add deploy group as a filter once that ships.
And finally, it could allow us to embed other information within the API call such as related namespace-level information, for example if the namespace has rate limits that should also be taken into account.
There was a problem hiding this comment.
Additional context. Versioning team originally shipped APIs within task queue, and Max asked us to make them top-level and simply refer to task queue from them. (Later on, we decided to switch to deploy group, but the top-level API concept remains.)
This feels like it could be analogous.
There was a problem hiding this comment.
Have we considered a top level scaling/tuning API instead that takes task queue types as a filter?
I am not sure. This is the Python review for the SDK API that already exists in Go and calls the already existing server API. I think existential questions concerning the API may need to be asked in a place with broader reach.
|
|
||
|
|
||
| @dataclass | ||
| class TaskQueueDescription: |
There was a problem hiding this comment.
How this struct will look like once the per-buildId info is added?
What was changed
Implementation of describe task queue as described at temporalio/features#525.
This is currently just a draft since it was opened early to get feedback.