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

Move POI and reports stream persistence logic into Store struct #22

Closed
wants to merge 14 commits into from

Conversation

neysofu
Copy link
Collaborator

@neysofu neysofu commented Mar 31, 2022

First step in moving all store-related logic into a single module to make logic clearer. No functional changes.

Depends on #11.

@neysofu neysofu changed the title Move some stream and db -related logic into Store struct Move POI and reports stream persistence logic into Store struct Mar 31, 2022
@neysofu neysofu force-pushed the filippo/refactor-store branch 2 times, most recently from 7a4b54c to 72bf484 Compare April 1, 2022 12:17
@neysofu neysofu force-pushed the filippo/refactor-store branch from 72bf484 to aa17745 Compare April 1, 2022 12:19
@neysofu
Copy link
Collaborator Author

neysofu commented Apr 5, 2022

All commits after 05ccd79 go towards closing #15.

Copy link
Collaborator

@Jannis Jannis left a 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.

@neysofu
Copy link
Collaborator Author

neysofu commented Apr 27, 2022

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.

@neysofu neysofu closed this Apr 13, 2023
@neysofu neysofu deleted the filippo/refactor-store branch April 24, 2023 13:46
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.

2 participants