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.

Sign up for GitHub

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

sync lex: #account firehose event; account status errors; and getAccountStatus endpoint #2263

Merged
merged 13 commits into from
May 29, 2024
Merged

sync lex: #account firehose event; account status errors; and getAccountStatus endpoint #2263

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
00ce729
subscribe repos lex: new #account event type
bnewbold Mar 2, 2024
a626d78
lex: getAccountStatus endpoint
bnewbold Mar 2, 2024
a78e623
lex: add account status errors to sync methods
bnewbold Mar 2, 2024
618c61e
tweak type of token union
dholms Mar 4, 2024
efbf94a
fix getAccountStatus parameter name
dholms Mar 4, 2024
a0a111b
Account -> Repo
dholms Mar 4, 2024
bd2ac10
codegen
dholms Mar 5, 2024
a9dbdbe
merge main
dholms May 20, 2024
673c7f6
schema tweaks
dholms May 21, 2024
2e122ef
update identity evt
dholms May 27, 2024
07ac1fe
add description to handle field
dholms May 27, 2024
b9e1700
status on listRepos & notate deprecated firehose events
dholms May 27, 2024
89b4bb0
Merge branch 'main' into bnewbold/account-event
dholms May 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion lexicons/com/atproto/sync/getBlob.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,14 @@
},
"output": {
"encoding": "*/*"
}
},
"errors": [
{ "name": "BlobNotFound" },
{ "name": "RepoNotFound" },
{ "name": "RepoTakendown" },
{ "name": "RepoSuspended" },
{ "name": "RepoDeactivated" }
]
}
}
}
9 changes: 8 additions & 1 deletion lexicons/com/atproto/sync/getBlocks.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,14 @@
},
"output": {
"encoding": "application/vnd.ipld.car"
}
},
"errors": [
{ "name": "BlockNotFound" },
{ "name": "RepoNotFound" },
{ "name": "RepoTakendown" },
{ "name": "RepoSuspended" },
{ "name": "RepoDeactivated" }
]
}
}
}
7 changes: 6 additions & 1 deletion lexicons/com/atproto/sync/getLatestCommit.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,12 @@
}
}
},
"errors": [{ "name": "RepoNotFound" }]
"errors": [
{ "name": "RepoNotFound" },
{ "name": "RepoTakendown" },
{ "name": "RepoSuspended" },
{ "name": "RepoDeactivated" }
]
}
}
}
9 changes: 8 additions & 1 deletion lexicons/com/atproto/sync/getRecord.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,14 @@
},
"output": {
"encoding": "application/vnd.ipld.car"
}
},
"errors": [
{ "name": "RecordNotFound" },
{ "name": "RepoNotFound" },
{ "name": "RepoTakendown" },
{ "name": "RepoSuspended" },
{ "name": "RepoDeactivated" }
]
}
}
}
8 changes: 7 additions & 1 deletion lexicons/com/atproto/sync/getRepo.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,13 @@
},
"output": {
"encoding": "application/vnd.ipld.car"
}
},
"errors": [
{ "name": "RepoNotFound" },
{ "name": "RepoTakendown" },
{ "name": "RepoSuspended" },
{ "name": "RepoDeactivated" }
]
}
}
}
42 changes: 42 additions & 0 deletions lexicons/com/atproto/sync/getRepoStatus.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
{
"lexicon": 1,
"id": "com.atproto.sync.getRepoStatus",
"defs": {
"main": {
"type": "query",
"description": "Get the hosting status for a repository, on this server. Expected to be implemented by PDS and Relay.",
"parameters": {
"type": "params",
"required": ["did"],
"properties": {
"did": {
"type": "string",
"format": "did",
"description": "The DID of the repo."
}
}
},
"output": {
"encoding": "application/json",
"schema": {
"type": "object",
"required": ["did", "active"],
"properties": {
"did": { "type": "string", "format": "did" },
"active": { "type": "boolean" },
"status": {
"type": "string",
"description": "If active=false, this optional field indicates a possible reason for why the account is not active. If active=false and no status is supplied, then the host makes no claim for why the repository is no longer being hosted.",
"knownValues": ["takendown", "suspended", "deactivated"]
},
"rev": {
"type": "string",
"description": "Optional field, the current rev of the repo, if active=true"
}
}
Comment on lines +24 to +36
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should results in sync.listRepos also include the hosting status, and perhaps generally have the same shape as this?

}
},
"errors": [{ "name": "RepoNotFound" }]
}
}
}
8 changes: 7 additions & 1 deletion lexicons/com/atproto/sync/listBlobs.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,13 @@
}
}
}
}
},
"errors": [
{ "name": "RepoNotFound" },
{ "name": "RepoTakendown" },
{ "name": "RepoSuspended" },
{ "name": "RepoDeactivated" }
]
}
}
}
27 changes: 26 additions & 1 deletion lexicons/com/atproto/sync/subscribeRepos.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
"refs": [
"#commit",
"#identity",
"#account",
"#handle",
"#migrate",
"#tombstone",
Expand Down Expand Up @@ -118,7 +119,31 @@
"properties": {
"seq": { "type": "integer" },
"did": { "type": "string", "format": "did" },
"time": { "type": "string", "format": "datetime" }
"time": { "type": "string", "format": "datetime" },
"handle": {
"type": "string",
"format": "handle",
"description": "The current handle for the account, or 'handle.invalid' if validation fails. This field is optional, might have been validated or passed-through from an upstream source. Semantics and behaviors for PDS vs Relay may evolve in the future; see atproto specs for more details."
}
}
},
"account": {
"type": "object",
"description": "Represents a change to an account's status on a host (eg, PDS or Relay). The semantics of this event are that the status is at the host which emitted the event, not necessarily that at the currently active PDS. Eg, a Relay takedown would emit a takedown with active=false, even if the PDS is still active.",
"required": ["seq", "did", "time", "active"],
"properties": {
"seq": { "type": "integer" },
"did": { "type": "string", "format": "did" },
"time": { "type": "string", "format": "datetime" },
"active": {
"type": "boolean",
"description": "Indicates that the account has a repository which can be fetched from the host that emitted this event."
},
"status": {
Comment on lines +138 to +142
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm alright with keeping active around, though I am not sure it is necessary. It allows a PDS to express some conflicting information, like setting a status with active=true.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

just to expand on the reasoning there a bit, I wanted to future-proof a bit against us adding additional status strings in the future. If simple infra implementations match on the status string to either make the account visible or not make the account visible (who knows what kind of weird match/switch control flow they might have), then adding a status code could result in the incorrect behavior. Making it a boolean flag makes the "is account visible or not" behavior very clear, with the status being more of an end-user-visible context and less of a high-stakes "is content redistributed at all".

still agree with your point, just wanted to document that bit of thinking

"type": "string",
Copy link
Collaborator

Choose a reason for hiding this comment

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

made a tweak here

with token values we normally just use a string type & list the token values in knownValues.

see com.atproto.moderation.defs for an example with report types

Copy link
Collaborator

@devinivy devinivy Mar 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

To be honest I am not even sure if we need tokens here— its possible it could be non-namespaced strings.

In my mind the value in tokens is that it allows extensibility by other namespaces, especially in cases where it's friendly for applications to be able to use values that will never make it into the lexicon's knownValues. I don't think that's necessarily needed or even desirable here in the sync endpoints. One reason we may not want that, is that it's nice to keep the values one-to-one with the error types on these sync endpoints, which are not namespaced. That allows you to understand the repo's status fully through the response of e.g. sync.getRepo.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's a good point - and I do tend to agree that this isn't a place that we're asking for extensibility on. Any changes should be part of the formal sync protocol 🤔

So maybe we just do hardcoded values?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That would be my vote! I still support keeping it an open union as a little safety net.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it feels like a place "hardcoded" would make sense. agree on keeping it open-union semantics.

"description": "If active=false, this optional field indicates a reason for why the account is not active.",
"knownValues": ["takendown", "suspended", "deleted", "deactivated"]
}
}
},
"handle": {
Expand Down
13 changes: 13 additions & 0 deletions packages/api/src/client/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ import * as ComAtprotoSyncGetHead from './types/com/atproto/sync/getHead'
import * as ComAtprotoSyncGetLatestCommit from './types/com/atproto/sync/getLatestCommit'
import * as ComAtprotoSyncGetRecord from './types/com/atproto/sync/getRecord'
import * as ComAtprotoSyncGetRepo from './types/com/atproto/sync/getRepo'
import * as ComAtprotoSyncGetRepoStatus from './types/com/atproto/sync/getRepoStatus'
import * as ComAtprotoSyncListBlobs from './types/com/atproto/sync/listBlobs'
import * as ComAtprotoSyncListRepos from './types/com/atproto/sync/listRepos'
import * as ComAtprotoSyncNotifyOfUpdate from './types/com/atproto/sync/notifyOfUpdate'
Expand Down Expand Up @@ -257,6 +258,7 @@ export * as ComAtprotoSyncGetHead from './types/com/atproto/sync/getHead'
export * as ComAtprotoSyncGetLatestCommit from './types/com/atproto/sync/getLatestCommit'
export * as ComAtprotoSyncGetRecord from './types/com/atproto/sync/getRecord'
export * as ComAtprotoSyncGetRepo from './types/com/atproto/sync/getRepo'
export * as ComAtprotoSyncGetRepoStatus from './types/com/atproto/sync/getRepoStatus'
export * as ComAtprotoSyncListBlobs from './types/com/atproto/sync/listBlobs'
export * as ComAtprotoSyncListRepos from './types/com/atproto/sync/listRepos'
export * as ComAtprotoSyncNotifyOfUpdate from './types/com/atproto/sync/notifyOfUpdate'
Expand Down Expand Up @@ -1229,6 +1231,17 @@ export class ComAtprotoSyncNS {
})
}

getRepoStatus(
params?: ComAtprotoSyncGetRepoStatus.QueryParams,
opts?: ComAtprotoSyncGetRepoStatus.CallOptions,
): Promise<ComAtprotoSyncGetRepoStatus.Response> {
return this._service.xrpc
.call('com.atproto.sync.getRepoStatus', params, undefined, opts)
.catch((e) => {
throw ComAtprotoSyncGetRepoStatus.toKnownErr(e)
})
}

listBlobs(
params?: ComAtprotoSyncListBlobs.QueryParams,
opts?: ComAtprotoSyncListBlobs.CallOptions,
Expand Down
Loading
Loading