Hi all, the @owner directive seems like a very useful tool for streamlining the review process and adding the “right” people to review schema changes in a PR, rather than having a long list of gatekeepers.
I’m curious if anyone has implemented something similar for Schema Proposals in Apollo Studio? My organization is looking to leverage this directive in a very similar way to what these docs say, but instead applying reviewers on a Github PR, applying reviewers on a Schema Proposal.
4 Likes
@greg-apollo gave a great talk on ownership. But not sure if Wayfair ties it to schema proposals. @Serey_Morm ?
1 Like
Using the GraphOS platform API this is possible today that you could script setting the reviewers on a proposal
mutation UpdateRequestedReviewers($input: UpdateRequestedReviewersInput!, $name: String!, $graphId: ID!) {
graph(id: $graphId) {
variant(name: $name) {
proposal {
... on ProposalMutation {
updateRequestedReviewers(input: $input) {
}
}
}
}
}
}
2 Likes
While I was still at Wayfair, the plan was to tie it into proposals. Ownership reviews at PR time, while useful, are a little late in the game. I believe it’s best to get the right people involved when designing the API rather than wait until it’s implemented to ask “what do you think?”
What Shane is suggesting was the plan.
- Tag ownership in the schema (so you know who should talk about what)
- Subscribe to changes to proposals
- Assign reviewers via the GraphOS Platform API based on which subgraph or what schema is changed
You can also enforce your own review limits (say, one reviewer from every changed subgraph has to approve) by subscribing to Proposal changes and the GraphOS Platform API.
1 Like
Thank you @greg-apollo & @shanemyrick! I had actually viewed your talk at summit this year, and this was the inspiration behind it all!
I will try this solution out and be sure to share it back with community 
1 Like
To help clarify here is a filled out mutation you could make
mutation UpdateRequestedReviewers {
graph(id: "graph id") {
variant(name: "graph variant aka proposal id") {
proposal {
... on ProposalMutation {
updateRequestedReviewers(input: {
reviewerUserIdsToAdd: ["123"]
reviewerUserIdsToRemove: ["123"]
}) {
__typename
... on Proposal {
displayName
id
status
}
... on PermissionError {
message
}
... on Error {
message
}
}
}
}
}
}
}
1 Like
I’ve tried making this exact mutation, supplying the proposal ID as the name input field, but it errors out on the variant level, saying it can’t return data for the proposal ID:
"message": "Exception while fetching data (/graph/variant) : Variant ddddcb97-c318-45e2-895d-54089b8268f6 not found",
"path": [
"graph",
"variant"
],
Could I be missing something here?
The proposal ID is something like p-1
I see, that makes sense. I’m now using the correct proposal ID.
Is there any reason this query would fail due to auth? I’m currently providing a Graph Admin API key as the X-API-KEY but getting this error
{
"message": "Exception while fetching data (/_entities[0]/memberships) : NOT_ALLOWED_BY_USER_ROLE",
"path": [
"graph",
"variant",
"proposal",
"proposal",
"backingVariant",
"graph",
"account",
"memberships"
],
"extensions": {
"serviceName": "identity",
"code": "DOWNSTREAM_SERVICE_ERROR"
}
},
I seemingly can only get this mutation to work if I use my own Personal API key as an org admin.
When using a Graph API Key that has Graph Admin permissions, I get the error above.
This documentation lays out that a Graph Admin or Contributor should have access to update reviewers: Members, Roles, and Permissions - Apollo GraphQL Docs
This is the level of access my graph has permission for with proposals:
@kylekrupp That appears to be a bug with the API keys, can you please raise a support request in the Help Center: https://support.apollographql.com/
Thanks for confirming @shanemyrick , adding the ticket ref here for posterity TSH-18159
Hey there @greg-apollo / @shanemyrick , I’ve been playing around with the Platform API a bit, but can’t seem to find a way to retrieve the schema diff on a given proposal.
Is there an obvious way to do that I may be missing? Or if not, is there a roundabout way I could programmatically generate the diff that is displayed in the Proposals view?
You should be able to find the diff in the ProposalImplementedChange.diffItem
field: Studio
Is the only way to retrieve diff data is if the proposal is implemented? My intention is to kick off this automated reviewer requesting when a proposal is opened
Update: for my intention, I put together this query that allows me to retrieve the core document in the proposal, and the core document of the latest launch for the variant. With this information I could infer the diff and cherry pick the info I need.
query Proposal($graphId: ID!, $name: String!) {
graph(id: $graphId) {
variant(name: $name) {
name
proposal {
latestRevision {
launch {
build {
result {
... on BuildSuccess {
coreSchema {
coreDocument
}
}
}
}
}
}
sourceVariant {
latestLaunch {
build {
result {
... on BuildSuccess {
coreSchema {
coreDocument
}
}
}
}
}
}
}
}
}
}
}
It looks like you can fetch the structured diff if you have the schema hash of the old schema from the previous version of the proposal to the current
query Diff($graphId2: ID!) {
graph(id: $graphId2) {
flatDiff(newSdlHash: "", oldSdlHash: "") {
}
}
}
2 Likes
Hi everyone, I have an architecture based on NestJS and Apollo Federation with microservices, where each microservice has its own dedicated database and is fully self-contained.
I spent days trying to find a way to assign an owner to mutations and queries, but without success. Eventually, I found a workaround using the @tag
directive, since the @owner
directive is only available for entities.
This is very limiting because, although the supergraph is a really cool concept, in my case — for permission and group management — it was necessary to define which microservice owned specific queries and mutations.
In general, this kind of “globalization” becomes a limitation in complex systems, unless you switch to an SDL-first architecture, which in my opinion is the wrong approach, as it would mean overriding NestJS and Apollo.
If in the future it became possible to declare the owner of queries and mutations, it would be greatly appreciated.
I’m not sure I’m following the problem. The @owner
directive should be a custom directive that you can apply to any schema node you wish (as long as it can accept a directive — so no directive definitions).
Here’s an example @owner
directive definition:
"Defines which graph owns a specific schema node."
directive @owner(
"The name of the graph that owns the schema node."
graph: String!
) on ARGUMENT_DEFINITION | ENUM | ENUM_VALUE | FIELD_DEFINITION | INPUT_OBJECT | INPUT_FIELD_DEFINITION | INTERFACE | OBJECT | SCALAR | UNION
That example @owner
directive definition makes it possible to apply the directive to pretty much every schema node in a subgraph (again, excluding directive definitions because they can’t have directives applied to them).
What I was trying to do is this — inside a resolver of one of my microservices:
@Query(() => [User], { name: 'findAllUsers' })
@Directive('@owner(name: "users")') // <- here
async findAllUsers(
@Args('groupId', { type: () => String, nullable: true }) groupId?: string,
): Promise<User[]> {
return this.usersService.findAllUsers(groupId);
}
By adding @Directive('@owner(name: "users")')
, I was expecting to see something like this in my generated schema:
type Query {
findAllUsers(groupId: String): [User!]! @owner(name: "users")
findOneUser(userId: String!, groupId: String): User! @owner(name: "users")
}
type Mutation {
createUser(createUserInput: CreateUserInput!): User! @owner(name: "users")
updateUser(updateUserInput: UpdateUserInput!): User! @owner(name: "users")
removeUser(userId: String!): DeleteUserOutput! @owner(name: "users")
}
But when I try to do this, I get an error like: “did you mean @defer?”
This feature would really help in tracking who the actual owners of specific queries and mutations are — for example, in my case, to define who can execute them based on group membership.