-
Notifications
You must be signed in to change notification settings - Fork 3
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
Move POI and reports stream persistence logic into Store
struct
#22
Conversation
Store
structStore
struct
7a4b54c
to
72bf484
Compare
72bf484
to
aa17745
Compare
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'll review at a higher level than usual.
I've managed to rebase this branch on top of main
. I've pushed the rebase here for reference: https://github.com/edgeandnode/graph-ixi/tree/jannis/refactor-store but I didn't want to overwrite anything in this PR just yet.
There are things I like in this PR and things I don't like. I like the store refactor itself, I think it's clean enough.
What I don't like:
SubgraphDeployment
becoming a struct with network information on it.- POI debug information should only be pulled in when there are differences in POIs but not for every POI ever, that's too expensive. What I was thinking is that this debug information was stored in the db separately from POIs and that it would be linked from POIReports that have a diverging block.
Understood, thank you for the thorough review of this and all the other PRs. It's been a while so I'd need to play around with it to regain the necessary context... and yeah, good point about only storing POIs differences. I hadn't thought about the explosion of space requirements. I think we might be able to sync on this once you get back and block oracle won't be so taxing on my schedule. |
First step in moving all store-related logic into a single module to make logic clearer. No functional changes.
Depends on #11.