Add /api/instances/list
#2199
Add /api/instances/list
#2199
Conversation
Add an API method for listing instances with filtering and pagination. This method will be used in the new Instances page in the UI. It is similar to the deprecated `/api/pools/list_instances` method, except it allows filtering by fleet and not by pool. Also add the `fleet_id` and `fleet_name` fields to the instance model so that the Instances page in the UI can display fleet names and provide links to fleet details.
| project_names: Optional[Container[str]], | ||
| fleet_ids: Optional[Iterable[uuid.UUID]], |
There was a problem hiding this comment.
Not sure if there is a good reason to use abstract types if lists are sufficient. Depending on abstract types puts extra limits on the implementation (e.g. may need to convert to list). I'd not use abstract types unless there is a specific reason.
There was a problem hiding this comment.
Conversely, depending on specific types puts extra limits on the caller, e.g. the caller may need to convert to list when in fact a set or a generator could work just as well.
It's hard to say which is more likely: changing the implementation in such a way that the abstract types are no longer sufficient, or adding new callers that prefer to use other specific types. I'd bet on the latter.
There was a problem hiding this comment.
Conversely, depending on specific types puts extra limits on the caller,
Agree, but in this specific case we talk about a service function that's written specifically for a router, so a router will likely be the only client.
Don't you think that following the approach you suggest with defaulting to abstract types we'll end up with lots of inconsistencies?
There was a problem hiding this comment.
we talk about a service function that's written specifically for a router, so a router will likely be the only client
It is also likely that Container and Iterable will be enough for the purposes of this function. And who knows, we may find other uses for this function as well.
There was a problem hiding this comment.
following the approach you suggest with defaulting to abstract types we'll end up with lots of inconsistencies
Do you mean actual type inconsistencies, when the declared type doesn't match the runtime type? This should be prevented by a type checker.
There was a problem hiding this comment.
I don't mean type correctness. I mean some functions will accept concrete types, some abstract types, and some a mixture of both. This will likely happen if you default to use abstract / generic types from now on.
There was a problem hiding this comment.
Then I don't think it's an issue. We will just trust the function's signature when calling it - if it expects a list, we pass a list; if expects an iterable, we pass any iterable.
if you default to use abstract types from now on
I've been using them all the time actually) And I see them in commits from other teammates as well.
There was a problem hiding this comment.
Ok, I don't insist on changing the types
Add an API method for listing instances with filtering and pagination. This method will be used in the new Instances page in the UI. It is similar to the deprecated
/api/pools/list_instancesmethod, except it allows filtering by fleet and not by pool.Also add the
fleet_idandfleet_namefields to the instance model so that the Instances page in the UI can display fleet names and provide links to fleet details.Part of #2133