Cull appview v2 down to frontend service #1982
Conversation
…lement mute/unmuting in mock dataplane
| import { AtpAgent } from '@atproto/api' | ||
| import { ImageInvalidator } from '../../src/image/invalidator' | ||
|
|
||
| describe('fuzzy matcher', () => { |
There was a problem hiding this comment.
hmmm ripping out automod stuff is definitely going to clash with my work, but hopefully by the time we're landing this branch, the separate automod service is in a good enough spot that it's alright 👌
There was a problem hiding this comment.
In my view we're ripping everything out in this PR to match the end state we want for the appview frontends. It's hard to imagine how automod functionality would remain here in v2, so I think we nix it and just handle the merge conflict in this branch at the next opportunity, even if it might be a little while.
| import { Database } from '../src' | ||
| import { PrimaryDatabase } from '../src/db' | ||
| import { Leader } from '../src/db/leader' | ||
| import { Database, PrimaryDatabase } from '../src' | ||
|
|
||
| describe('db', () => { |
There was a problem hiding this comment.
since this is only used for mocks, you can probably just delete this whole test 🤷♂️
There was a problem hiding this comment.
actually same with several of these - did-cache, duplicate-records, handle-invalidation, etc
There was a problem hiding this comment.
Sort of torn on this! If the code is still around and is important for correctness of the other tests, maybe it's worth keeping them around in some form. Pretty sure it will remain low cost to maintain. Perhaps they could just be collapsed into a single file and condensed a bit.
| "val": "test-label-2", | ||
| }, | ||
| ], | ||
| "labels": Array [], |
There was a problem hiding this comment.
hmmm these were good integration tests that labels were being hydrated correctly - think we could just add these labels directly in the seed scripts?
not a huge deal, but we don't currently have separate tests for label hydration 😬
There was a problem hiding this comment.
Seems like a good idea! Will take a look at that.
| @@ -19,11 +18,16 @@ describe('feedgen proxy view', () => { | |||
| beforeAll(async () => { | |||
| network = await TestNetwork.create({ | |||
| dbPostgresSchema: 'proxy_feedgen', | |||
| bsky: { algos: makeAlgos(feedUri.host) }, | |||
| bsky: { | |||
| // @TODO consider using makeAlgos() here if the appview begins supporting any feeds out of the box | |||
There was a problem hiding this comment.
i might actually just cull all the tests related to this
| import { NotificationServer } from '../src/notifications' | ||
| import { Database } from '../src' | ||
|
|
||
| describe('notification server', () => { |
There was a problem hiding this comment.
any idea the future of this stuff? it'll be handled by the dataplane indexer i guess? should just make a note so we don't forget about it 😅
There was a problem hiding this comment.
Indeedy, looking like it will move into the indexer. I have notes on it, wont lose this!
There was a problem hiding this comment.
High level comment, but I also think we can clean up a lot of unused stuff from the database now that it's just used for mocks (such as the database coordinator, background queue for indexing, etc).
But also we probably want to eventually just convert this to sqlite, so I don't think it's necessary to take a pass on all that right now
There was a problem hiding this comment.
Looks good! & stoked to get to cut all this out 🕺
I wasn't super thorough in my review, but I scanned the whole PR & grabbed the branch to poke around through the important bits.
The only big thing I think I'll miss is label hydration in all the snaps, it'd be great to find another way to get that in there if possible.
Besides that, I think there's just other code that can be culled as well, but it's not super pressing & we can probably save that for a clean up pass once it's all said & done
|
@dholms made some changes!
|
Reduces the appview down to only use the dataplane service, e.g. dropping the direct dependency on postgres. Removes all functionality not related to the appview frontends, i.e. indexing, ingestion, and the daemon. Some indexing functionality has been preserved in
src/data-plane/serverwhich is a postgres implementation of the dataplane.