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.

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

Add backfilling to search backed by GORM #294

Merged
merged 7 commits into from
Sep 11, 2023
Merged

Add backfilling to search backed by GORM #294

merged 7 commits into from
Sep 11, 2023

Conversation

ericvolp12
Copy link
Collaborator

Search will need to backfill regularly when rebuilding indexes.

I've adopted a backfill pattern I created a few weeks ago into indigo and updated Search's firehose consuming to adopt the backfilling pattern.

backfill/backfill.go Outdated Show resolved Hide resolved
@bnewbold
Copy link
Collaborator

I haven't reviewed in depth, but i'm pretty darn excited about having a generic backfill mechanism like this! Great work.

backfill/backfill.go Outdated Show resolved Hide resolved
return
}

req.Header.Set("Accept", "application/vnd.ipld.car")
Copy link
Collaborator

Choose a reason for hiding this comment

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

all this feels like it wants to be abstracted out a little. i assume you have a non-lexicon 'fetch repo' endpoint youre hitting?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the lexicon GetCheckout endpoint, which we could do with an XRPC client and such but the body is just a raw CAR payload and we don't gain anything by using the lexicon and XRPC boilerplate here really... I don't even need this header to be honest.

backfill/backfill.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@whyrusleeping whyrusleeping left a comment

Choose a reason for hiding this comment

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

a few comments, the code could be cleaned up a bit, but i wont let that block anything

@ericvolp12 ericvolp12 changed the title Draft: Add backfilling to search backed by GORM Add backfilling to search backed by GORM Sep 11, 2023
@ericvolp12 ericvolp12 merged commit cdc0baf into main Sep 11, 2023
6 checks passed
@ericvolp12 ericvolp12 deleted the search_backfill branch September 11, 2023 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants