Skip to content

Add ListTeamsIncludingProperties for batched team metadata reads - #643

Open
saditya370 wants to merge 1 commit into
mainfrom
aditya/OPS-27/list-teams-including-properties
Open

Add ListTeamsIncludingProperties for batched team metadata reads#643
saditya370 wants to merge 1 commit into
mainfrom
aditya/OPS-27/list-teams-including-properties

Conversation

@saditya370

Copy link
Copy Markdown
Contributor

Problem

Team.Properties is tagged graphql:"-", so it is never selected by generated
queries. Any caller that wants properties for a list of teams has to call
(*Team).GetProperties per team.

For the Terraform provider's opslevel_teams data source that is one request
per team. Measured against a 263-team account, the per-team approach issued
~266 requests and took 26 seconds, close enough to the provider's 30s
client timeout that one of two runs died mid-read. It is not viable at
customer scale.

Solution

ListTeamsIncludingProperties selects the properties connection inline
alongside the team list, so listing every team costs one request per page
instead of one per team. Same account, same data: 3 requests, under a
second
end to end through the provider.

TeamWithProperties embeds Team and redeclares Properties with a graphql
tag. Go field shadowing means the embedded graphql:"-" field is skipped and
ours is selected, so the query asks for properties exactly once. That is
subtle enough to be worth a guard, so TestListTeamsIncludingProperties
asserts the full constructed query string.

Because the test registers a single request and the autopilot harness fails
on any request it was not told to expect, a per-team fan-out regression would
surface as an unregistered call rather than silently passing.

Tags and memberships are completed by the existing Team.Hydrate, which issues
no request unless a connection actually spills past its first page. The inlined
properties connection is selected without pagination arguments, so the API
returns its first 100; a team holding more than that is topped up individually.

Alternatives considered

  • Remove graphql:"-" from Team.Properties. Simplest, but it would make
    every existing Team query heavier for every consumer. Rejected.
  • Implement this in the Terraform provider. Works, but couples the provider
    to an opslevel-go internal modeling decision and leaves other consumers
    (opslevel-mcp calls ListTeams too) to reinvent it. Rejected in favour of
    fixing it at the client layer.
  • Naming. Deliberately not ListTeamsWithProperties — every existing
    ListXWith* in this repo means filtered by (ListTeamsWithManager,
    ListServicesWithTier), so that name would read as "teams that have
    properties".

No regression by construction: the diff is 146 insertions and 0 deletions.
Not one existing line changed.

Noted but not fixed here

ListTeams only calls Hydrate on nodes from its recursive call, so teams on
the first page are never hydrated. With a default page size of 500, that
means most accounts get no hydration at all: Responsibilities stays
HTML-escaped and TotalCount stays 0. The same pattern appears in 17 places
across 5 files. Filing separately, since fixing it changes observable values
for every existing consumer and does not belong in an additive PR.

Checklist

  • I have run this code, and it appears to resolve the stated issue.
  • This PR does not reduce total test coverage
  • This PR has no user interface changes or has already received approval from product management to change the interface.
  • Does this change require a Terraform schema change?
    • Yes — OpsLevel/terraform-provider-opslevel PR for OPS-27, which consumes this and must merge after this releases.
  • Make a changie entry that explains the customer facing outcome of this change

@saditya370
saditya370 force-pushed the aditya/OPS-27/list-teams-including-properties branch from d3deb22 to 0909c68 Compare August 25, 2026 12:22
Team.Properties is excluded from generated queries with `graphql:"-"`, so a
caller needing properties for many teams pays one extra request per team.

Add a TeamWithProperties type that embeds Team and redeclares Properties with
a graphql tag, selecting the connection inline, plus
ListTeamsIncludingProperties which returns every team with its tags and
properties at one request per page. Tags and memberships are completed by the
existing Team.Hydrate; a team holding more than 100 properties is topped up on
its own rather than charging every team an extra request.

Purely additive. Team.Properties, ListTeams, and Hydrate are untouched, so
existing callers execute identical code paths.
@saditya370
saditya370 force-pushed the aditya/OPS-27/list-teams-including-properties branch from 0909c68 to 3021749 Compare August 25, 2026 12:24
@saditya370 saditya370 self-assigned this Aug 25, 2026
Comment thread team.go
//
// The inlined properties connection is selected without pagination arguments, so the API
// returns its first 100 entries; any team holding more than that is topped up on its own.
func (client *Client) ListTeamsIncludingProperties(variables *PayloadVariables) ([]TeamWithProperties, error) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think that this solution is very composable. If we want to extend what teams returns, we'll have to deprecate this and replace it with something else. I'll come up with a recommendation.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think that instead of doing ListTeamsIncludingProperties or hydrating properties per team, we can remove the properties opt out & hydrate them per page instead of per team.

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