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

chore(sequencer): remove misplaced logs #1892

Merged
merged 2 commits into from
Jan 9, 2025

Conversation

SuperFluffy
Copy link
Member

@SuperFluffy SuperFluffy commented Jan 8, 2025

Summary

Remove events from Sequencer::run_until_stopped
that report on whether whether a state DB will be
opened or created.

Background

Cnidarium already reports which paths it will read.

More egregious however is that
Sequencer::run_until_stopped is the wrong place
to emit these events because it does not have
authority over which actions cnidarium will take.
Should the constructor ever change, then the events
will be immediately out of whack, which will lead
to confusion.

Changes

  • Remove state storage creation events from Sequencer::run_until_stopped.

Testing

No testing necessary. These are just some limited events.

Changelogs

Changelogs updated.

@github-actions github-actions bot added the sequencer pertaining to the astria-sequencer crate label Jan 8, 2025
@SuperFluffy SuperFluffy force-pushed the superfluffy/remove-confusing-logs branch from a5c2e4d to 0d228bf Compare January 8, 2025 14:18
Sequencer checks if the storage path exists and
then reports on actions that it thinks what the
call `cnidarium::Storage::load` will do.

However, on the one hand cnidarium itself already
reports that information. On the other hand,
`Sequencer::run_until_stopped` is the wrong place
to emit these events because it does not have
authority over which actions cnidarium will take.
Should the construct ever change, then the events
will be immediately out of whack, which will lead
to confusion.
@SuperFluffy SuperFluffy force-pushed the superfluffy/remove-confusing-logs branch from 0d228bf to 85a1621 Compare January 8, 2025 14:18
@SuperFluffy SuperFluffy marked this pull request as ready for review January 8, 2025 14:21
@SuperFluffy SuperFluffy requested a review from a team as a code owner January 8, 2025 14:21
@SuperFluffy SuperFluffy requested a review from noot January 8, 2025 14:21
@SuperFluffy SuperFluffy enabled auto-merge January 8, 2025 14:25
@SuperFluffy SuperFluffy added this pull request to the merge queue Jan 8, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 8, 2025
@SuperFluffy SuperFluffy added this pull request to the merge queue Jan 8, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 8, 2025
@SuperFluffy SuperFluffy added this pull request to the merge queue Jan 8, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 8, 2025
@SuperFluffy SuperFluffy added this pull request to the merge queue Jan 8, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 8, 2025
@SuperFluffy SuperFluffy enabled auto-merge January 9, 2025 13:39
@SuperFluffy SuperFluffy added this pull request to the merge queue Jan 9, 2025
Merged via the queue into main with commit a19b822 Jan 9, 2025
46 checks passed
@SuperFluffy SuperFluffy deleted the superfluffy/remove-confusing-logs branch January 9, 2025 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sequencer pertaining to the astria-sequencer crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants