-
Notifications
You must be signed in to change notification settings - Fork 554
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.
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
Sequencer recovery #2519
base: main
Are you sure you want to change the base?
Sequencer recovery #2519
Conversation
packages/common-web/src/arrays.ts
Outdated
export const filterDefined = <T>(arr: (T | undefined)[]): T[] => { | ||
return arr.filter((item) => item !== undefined) as T[] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to take a little extra care now to include changesets for changes to deps, so that the PDS distro will rely on a proper published version of its deps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ended up just scrapping this util func and using a type annotation instead 👌
(most recent TS version actually fixes the need for this 👀)
packages/pds/src/scripts/common.ts
Outdated
if (ctx.entrywayAdminAgent) { | ||
await ctx.entrywayAdminAgent.api.com.atproto.admin.updateAccountSigningKey({ | ||
did, | ||
signingKey: updateTo.did(), | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we'll always land in this case since entrywayAdminAgent
exists when there's an entryway admin token, which fallsback to env.adminPassword
, which should always be set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's also contingent on having an entryway cfg as well which is the main thing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ended up being more straightforward than I expected, nice! Made it look easy.
One idea that comes to mind is that it's a little bit of a shame the whole accounts table is processed in a single process, and it would be nice if we could break that up. I think all we would need is for the scripts to support a partitioning scheme, e.g. an option to say "run this for partition #1 of 10". Then we'd run the script in 10 processes and get a little more juice.
Adds script to pds for performing recovery based off of the sequencer database: