-
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
Appview proxy #827
Merged
Merged
Appview proxy #827
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dholms
commented
Apr 18, 2023
@@ -39,7 +39,7 @@ export const composeFeed = async ( | |||
if (post && originator) { | |||
let reasonType: string | undefined | |||
if (row.type === 'repost') { | |||
reasonType = 'app.bsky.feed.feedViewPost#reasonRepost' |
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.
this one got missed in lex refactor
Closed
devinivy
approved these changes
Apr 24, 2023
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.
Oh yeah!
Did caching
Simple service auth
mloar
pushed a commit
to mloar/atproto
that referenced
this pull request
Nov 15, 2023
* fix up a couple of tsc errors in app view merge * wip * simple proxy * use dev-env for appview tests * process all in blob resolver * another test fix * wip * copy proxied tests & add mutes to getFollows/getFollowers * tidy & add mutes to likes * more routes + getAuthorFeed tests * more testing * tests for feed views * thread testing * finished tests for threads * temporarily skip some tests * cleaning up & updating test names * separate db schema for appview * rearrange * typo * add notifications * re-enable notifs on pds appview * update schemas * updated some bsky snaps * wip * refactor did-resolver * clean up deps * some fixups + caching utilities * fix up & move to appCtx * neat its working * update bsky tests to new auth * rm unused pds config var * tidy * check exp in seconds * cache dids in postgres * add migration & did-cache * start tests * couple helpers around cache invalidation * fix expired check * wip * change cache semantics * did cache testing * do some cache revalidation in indexing * fix config * fix issue w did-resolver test-env prototype * use map instead of record * stale + expired * tests * clear entry method * fix up build * expired dids * clear missing dids * better verify payload * bump test timeout * fix notifs test * fix up proxied actor search tests * update snaps to include labels * fix dev env * fix up moderation route auth * fix more auth headers * fix auth on getPosts * increase jest timeouts * fix snaps
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proxying requests from PDS to AppView
For all view requests, we forward the request to the configured appview, then hydrate mutes into the response before sending it back to the client.
Some implementation notes:
I've made the PDS binary either "everything or nothing proxied to appview". We can set this up per-user, but would require
some extra user state kicking around & a DB hit at the start of each request. I'm not sure if we'll want to do this given we're going to be deploying & testing in the sandbox.
Each PDS only has one configured AppView. In the future we can let a user set their preferred AppView.
I disabled the actor search tests since they kept timing out. In general, our bulk user data tests do not perform well & we probably need to do something about that (I think that this is an scrypt thing & we might be able to fix by avoiding scrypt in dev envs). If you have other ideas plz let me know.
That being said, I am concerned about the performance of
searchActorsTypeahead
being proxied to the appview. We may want to handle that some other way...For testing, I just copied over our view tests into a new
proxied
folder. This has led to a decent bit of duplicated logic/tests, but as these services come together & become more stable, we should be able to start cleaning it up. For now, since both paths are supported, I feel that it's important to test both paths. Let me know if there is anything you would do in the meantime.For testing, I also switched the pds to default test with postgres. There is a new
yarn test:sqlite
that will use sqlite & not run the proxied tests (which rely on postgres because of appview)Future work: