op-chain-ops: add logging to db migration #3927
Conversation
|
tynes
requested review from
mslipper,
protolambda,
smartcontracts and
trianglesphere
There was a problem hiding this comment.
Changes LGTM, but it would be nice to not use the global logger as much. In op-e2e testing it has been very useful to decorate more localized loggers with different names like verifier/sequencer etc. when we have different instances running. And we can direct the log output with a test-logger, instead of having the unorganized std-out throughout parallel tests.
This is a good point, I didn't want to change the interfaces in a bunch of places to pass through a logger. Would require a bit of a refactor to achieve what you are saying |
|
Hey @tynes! This PR has merge conflicts. Please fix them before continuing review. |
|
This PR has been added to the merge queue, and will be merged soon. |
|
Hey @tynes, this pull request failed to merge and has been dequeued from the merge train. If you believe your PR failed in the merge train because of a flaky test, requeue it by commenting with |
This commit adds additional logging to the database migration codepath. This is helpful for understanding what is happening in the code, to understand how much of the migration has already happened. It may also help catch programmer/configuration errors, although we should have testing to catch that and not rely on the programmer watching the logs to catch issues.
tynes
force-pushed
the
chain-ops/logging
branch
from
f067c3a to
3698b7e
Compare
|
This PR has been added to the merge queue, and will be merged soon. |
|
This PR is next in line to be merged, and will be merged as soon as checks pass. |
|
Hey @tynes, this pull request failed to merge and has been dequeued from the merge train. If you believe your PR failed in the merge train because of a flaky test, requeue it by commenting with |
|
@Mergifyio requeue |
✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically |
|
This PR has been added to the merge queue, and will be merged soon. |
|
This PR is next in line to be merged, and will be merged as soon as checks pass. |
4 participants
Description
This commit adds additional logging to the database migration codepath. This is helpful for understanding what is happening in the code, to understand how much of the migration has already happened.
It may also help catch programmer/configuration errors, although we should have testing to catch that and not rely on the programmer watching the logs to catch issues.