-
Notifications
You must be signed in to change notification settings - Fork 0
refactor: remove BlocksBeingFetched state #210
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
Conversation
Gipphe
left a comment
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 PR is arguably somewhat muddled. We've got 3 disparate set of changes happening:
- Removal of
BlocksBeingFetched. - Partial introduction of
withNamespace. - Rework of what
Hoard.Monitoringlogs.
Would you mind splitting these up into at least 2, if not 3, separate PRs? Just ping me on Slack the moment you've got them set up, and I'll provide a swift review to keep the pace going!
Other than that, this is looking very good! It'd be nice to remove the broken implementation of BlocksBeingFetched, and I love what we now get out of Hoard.Monitoring.
|
🎉 All dependencies have been resolved ! |
0da6d24 to
8e7a2a3
Compare
Simplifies the BlockFetch logic by removing the BlocksBeingFetched tracking state. Also improves logging with namespaces and more detailed messages, and enhances monitoring output.
8e7a2a3 to
65f9fc9
Compare
| refreshImmutableTip :: (Log :> es, NodeToClient :> es, State HoardState :> es) => Eff es () | ||
| refreshImmutableTip = do | ||
| Log.debug "Fetching immutable tip from cardano-node..." | ||
| Log.info "Fetching immutable tip from cardano-node..." |
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.
Depends on #209
Simplifies
BlockFetchlogic by removing theBlocksBeingFetchedtracking state and theQSemlogic. Also improves logging with namespaces and more detailed messages, and adds some more info to the monitoring output.Let's keep things simple for now so we can iterate faster. We can re-add these later when needed.