Finalize PDS in-process AppView removal #1198

dholms · 2023-06-12T22:36:41Z

Finalizing the work of removing the AppView from the PDS & syncing this branch (which is being used for the current PDS distribution in sandbox) up with main

Refactor config
Wipe migrations
Tidying/clean up

bnewbold

Left minor devex-related comments. I would lean towards just merging this huge effort and then address any of those in a follow-on, instead of trying to resolve them first.

In that category: make run-dev-pds doesn't work on this branch; I think (and make run-dev-bsky) should not maybe point to services/pds/ and services/bsky/?

bnewbold · 2023-09-27T01:44:18Z

packages/pds/.env.example

@@ -0,0 +1,24 @@
+# See more env options in src/config/env.ts


Should we merge this file with the existing packages/pds/example.dev.env? which gets auto-copied for use with make run-dev-pds. I'd otherwise be worried this .env.example will be hard to find (default hidden dotfile) and get stale.

If we did that, would need to ensure that the config actually works for local dev, instead of sandbox. I think just commenting out partial strings would work.

Yup yup that makes sense 👍

i actually forgot we already had an .env.example

alright I actually removed the make run-dev-pds & make run-dev-bsky commands as well as their env files

Right now, the best way to run these services in dev is in the dev-env the packages themselves aren't set up very well with service entry files. So I also moved over the dotenv code to dev-env so that it's env can be adjusted

I don't think we were using those Make commands at all during development, but lmk if this affects your ability to develop.

In the future, it might be worth taking another pass at refactoring running each of theses services in dev mode

I do use those make commands fairly often, for the PDS and appview, distinct from dev-env. my workflow is basically to run make run-dev-* in a bunch of different terminals and a bunch of repos, while editing 1-2 code bases.

but I can try to get all that working in a follow-up PR, don't need to block this one on that side feature

packages/pds/src/config/env.ts

devinivy

Looking pretty solid, pretty much just left a few misc questions and notes 💎

packages/dev-env/src/network-no-appview.ts

devinivy · 2023-09-27T21:48:19Z

packages/pds/src/api/app/bsky/preferences/searchActorsTypeahead.ts

+    handler: async ({ params, auth }) => {
+      const requester = auth.credentials.did
+      const res =
+        await ctx.appViewAgent.api.app.bsky.actor.searchActorsTypeahead(
+          params,
+          await ctx.serviceAuthHeaders(requester),
+        )
+      return {
+        encoding: 'application/json',
+        body: res.data,
+      }


Now that there's no logic in many of these routes, it's really tempting to create a lighter-weight passthrough proxy handler. Can just avoid a fair amount of parsing, validation, memory usage, etc.

I agree 🤔

Could see about trying to wire something simple up - would be nice

devinivy · 2023-09-27T22:00:21Z

packages/pds/src/config/config.ts

+
+  const bskyAppViewCfg: ServerConfig['bskyAppView'] = {
+    url: env.bskyAppViewUrl ?? 'https://api.bsky-sandbox.dev',
+    did: env.bskyAppViewDid ?? 'did:plc:abc', // get real did


If the user sets bskyAppViewUrl but not bskyAppViewDid this could get out of whack, but this should be correct in the more likely case that neither are set:

Suggested change

did: env.bskyAppViewDid ?? 'did:plc:abc', // get real did

did: env.bskyAppViewDid ?? 'did:web:api.bsky-sandbox.dev',

I actually may just get rid of the defaults & throw if not included 🤔

This was added really just for ease of running in sandbox

devinivy · 2023-09-27T23:04:09Z

packages/pds/src/db/migrations/20230613T164932261Z-init.ts

+  await db.schema
+    .createTable('app_migration')
+    .addColumn('id', 'varchar', (col) => col.primaryKey())
+    .addColumn('success', 'int2', (col) => col.notNull().defaultTo(0))
+    .addColumn('completedAt', 'varchar', (col) => col)
+    .execute()


I wonder if we can get rid of this table and the app migration system.

maybe? it might be helpful with some automatic migrations for the pds distribution in the future tho 🤔

I think the runtime flag table is asking for it more tbh

devinivy · 2023-09-27T23:05:04Z

packages/pds/src/db/migrations/20230613T164932261Z-init.ts

+    .addColumn('did', 'varchar', (col) => col.primaryKey())
+    .addColumn('root', 'varchar', (col) => col.notNull())
+    .addColumn('indexedAt', 'varchar', (col) => col.notNull())
+    .addColumn('takedownId', 'varchar')


I think we may need to run a manual migration for the live service to change these cols to varchars.

Oh good snag 🤔

Would we rather these be varchars or integers? Does it matter?

I can't quite remember why we changed it to a varchar back in the day

I think it was so that you can store any identifier there, i.e. if the takedown came from some external system that didn't use integers to identify their actions. Pretty sure it was foreseeing removing some of the moderation machinery from the PDS, making that field less coupled to the current implementation.

Overall it doesn't matter a ton either way, though 👍

packages/pds/tests/proxied/views.test.ts

packages/pds/src/storage/disk-blobstore.ts

devinivy · 2023-09-27T23:35:20Z

packages/pds/tests/_util.ts

-      ? Database.postgres({
-          url: cfg.dbPostgresUrl,
-          schema: cfg.dbPostgresSchema,
-          txLockNonce: cfg.dbTxLockNonce,


I can't think of anything that allows us to drop this—is it possible we need to bring it back? Or maybe I missed the change!

It got moved to AppContext 👍

services/pds/index.js

packages/pds/src/config/config.ts

* refactor pds tests to use dev env * refactor bsky tests * fix pds test * tidy bsky tests

* switch takedown ids back to ints, consistent with live pds * tidy/fix migration * update migration for sqlite

The simplify-pds branch was deleted on 2023-10-02 after it was merged into main. Refs: <bluesky-social/atproto#1198>

* rm tables * rm event-stream & proxied * Remove appview services, move label service to pds * only proxy appview stuff * delete more tables * Start removing message dispatched from pds * more syncing-up removal of message dispatcher in pds * merged * remove feedgens from pds, remove getPopular * remove unused image helper from pds * fixing compiler errors * clean up sharp * rm label service * first pass on cleaning up tests * fix up a bunch of tests * moderation view tests * last admin tests * got a lil overzealous in deletes * clean up unused cfg * clean up label table * simplify admin repo search query/logic * tidy pds entrypoint * in-progress pds config changes * cfg fiddling * finish cleaning up cfg/ctx * comments * building * pds prefix on env * test env * collapse pds migrations down into a single migration * fix up dev-env * tidy * cleanup * fix pds admin tests * fix handle test * fix pds proxy tests * fix subscribe repos test * fix sqlite config in pds tests * add sqlite clause in sequencer-leader * fix actor search w/ sqlite on pds * fixes * fix dev env build * update pds service entrypoint * simple env example * make takedown ids opaque identifiers in the pds * use pds routes for api tests * update pds dockerfile with volume and correct port env var * add a couple env vars to example * add comments to env example * @atproto/pds 0.2.0-beta.0 * @atproto/aws 0.0.1-beta.0 * appview did * @atproto/aws 0.0.1 * enable logs by default * update env example * bugfixing sandbox issues * consistency in pds env var name for appview url * log on pds start and stop, configure version at runtime * @atproto/pds 0.2.0-beta.1 * fix semver matching for pds beta version * v0.2.0-beta.2 * default invites to being not required * fix flaky test * limit db connections in tests * publish 0.2.0-beta.d3 * fix invite required parsing * @atproto/pds 0.2.0-beta.5 * Proxy getPopularFeedGenerators on simplified pds (bluesky-social#1222) proxy getPopularFeedGenerators on pds Co-authored-by: dholms <[email protected]> * tidy migrations * fix service entry * bump version * change auth order * bump version * bump version * add upgradeRepoVersion & fallback url for cdn * bump version * merging * merge pds * building dev-env * merging tests * merge service entry * test fixing * tidy * fix admin search * tidy * tidy * add snap for getListFeed * add backup nameserver cfg * tidy + pr feedback * tidy * tidy env * bit more * re-add dotenv to root package.json * fix dep * build branch * fix tests * Refactor tests to make better use of dev-env (bluesky-social#1690) * refactor pds tests to use dev env * refactor bsky tests * fix pds test * tidy bsky tests * build pds correctly * fix entry point * default logging to false (for now) * format service entry * Switch takedown ids back to ints on pds distribution (bluesky-social#1694) * switch takedown ids back to ints, consistent with live pds * tidy/fix migration * update migration for sqlite * export moderation action reversal * takedown tests * dont build branch --------- Co-authored-by: Devin Ivy <[email protected]>

rm tables

e71a5e3

dholms force-pushed the simplify-pds branch from ec38b70 to e71a5e3 Compare June 12, 2023 22:39

dholms and others added 28 commits June 12, 2023 17:39

rm event-stream & proxied

a141d44

Remove appview services, move label service to pds

3b32f2b

only proxy appview stuff

44e4715

delete more tables

e600fab

Start removing message dispatched from pds

dc240c2

more syncing-up removal of message dispatcher in pds

ac35923

merged

b241fba

remove feedgens from pds, remove getPopular

1b10634

remove unused image helper from pds

d318f3e

fixing compiler errors

49010ed

clean up sharp

b458780

rm label service

c9c561d

first pass on cleaning up tests

2774c17

fix up a bunch of tests

69f3727

moderation view tests

9c4667d

last admin tests

6940a76

got a lil overzealous in deletes

810273e

clean up unused cfg

b228090

clean up label table

5ad2e09

simplify admin repo search query/logic

1ad57ba

tidy pds entrypoint

12cfdfd

in-progress pds config changes

5172554

cfg fiddling

b7b3d60

finish cleaning up cfg/ctx

dcdeeaa

comments

68a2daf

building

bde6307

pds prefix on env

def6dc4

test env

03b4b88

tidy

58d527b

dholms changed the title ~~Remove in-process AppView from PDS~~ Finalize PDS in-process AppView removal Sep 27, 2023

dholms marked this pull request as ready for review September 27, 2023 01:24

add snap for getListFeed

c3f4860

bnewbold reviewed Sep 27, 2023

View reviewed changes

add backup nameserver cfg

1c30e8c

devinivy approved these changes Sep 27, 2023

View reviewed changes

dholms and others added 19 commits September 28, 2023 11:45

tidy + pr feedback

b2864ca

tidy

75f3653

tidy env

5603a19

bit more

6dd27aa

re-add dotenv to root package.json

497d100

merge

046f82c

fix dep

fb75e28

build branch

20b97c2

fix tests

1134fd8

Refactor tests to make better use of dev-env (#1690)

5b2f4d1

* refactor pds tests to use dev env * refactor bsky tests * fix pds test * tidy bsky tests

build pds correctly

c7285a8

fix entry point

7506d5d

default logging to false (for now)

3efaa1d

format service entry

d3e194f

Switch takedown ids back to ints on pds distribution (#1694)

f2d06c4

* switch takedown ids back to ints, consistent with live pds * tidy/fix migration * update migration for sqlite

export moderation action reversal

7e5920f

Merge branch 'main' into simplify-pds

dc0360f

takedown tests

4fef68b

dont build branch

fc193ac

dholms merged commit d664b51 into main Oct 2, 2023
10 checks passed

dholms deleted the simplify-pds branch October 2, 2023 18:27

edavis added a commit to edavis/pds that referenced this pull request Oct 15, 2023

README.md: point to env.ts in atproto's main branch, not simplify-pds

b338e73

The simplify-pds branch was deleted on 2023-10-02 after it was merged into main. Refs: <bluesky-social/atproto#1198>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Finalize PDS in-process AppView removal #1198

Finalize PDS in-process AppView removal #1198

dholms commented Jun 12, 2023 •

edited

Loading

bnewbold left a comment

bnewbold Sep 27, 2023

dholms Sep 27, 2023

dholms Sep 28, 2023

bnewbold Sep 28, 2023

devinivy left a comment

devinivy Sep 27, 2023

dholms Sep 28, 2023

devinivy Sep 27, 2023

dholms Sep 28, 2023

devinivy Sep 27, 2023

dholms Sep 28, 2023

devinivy Sep 27, 2023

dholms Sep 28, 2023

dholms Sep 28, 2023

devinivy Sep 28, 2023

devinivy Sep 27, 2023

dholms Sep 28, 2023

	did: env.bskyAppViewDid ?? 'did:plc:abc', // get real did
	did: env.bskyAppViewDid ?? 'did:web:api.bsky-sandbox.dev',

Finalize PDS in-process AppView removal #1198

Finalize PDS in-process AppView removal #1198

Conversation

dholms commented Jun 12, 2023 • edited Loading

bnewbold left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devinivy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dholms commented Jun 12, 2023 •

edited

Loading