feat: blob info V2 for tracking pooled blobs#3109
Conversation
2e7f39e to
c1d5617
Compare
|
Quoting local codex:
Yes. For a631793 (refactor: move PerObjectBlobInfoV1 to blob_info_v1.rs), I verified it is a no-op refactor by inspection. The diff only:
I did not find any logic change in that commit. For 50f7263 (feat: introduce BlobInfoV2 and PerObjectBlobInfoV2 as copies of V1), the answer is also effectively yes, with one small
The commit also adds the expected module/enum plumbing in crates/walrus-service/src/node/storage/blob_info/ |
c1d5617 to
d50f517
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d50f5176b9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
crates/walrus-service/src/node/storage/blob_info/blob_info_v2.rs
Outdated
Show resolved
Hide resolved
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Co-Authored-By: Claude Opus 4.6 <[email protected]>
6a54786 to
2974cbf
Compare
Co-Authored-By: Claude Opus 4.6 <[email protected]>
2974cbf to
6a19cf9
Compare
mlegner
left a comment
There was a problem hiding this comment.
Thank you so much for implementing this and structuring it in such a reviewer-friendly way! 🙏
I confirmed that the first two commits are essentially no-ops and then only reviewed the third commit.
Overall, everything looks sensible to me. I have a few detailed suggestions and one larger one concerning the PerObjectBlobInfoV2.
crates/walrus-service/src/node/storage/blob_info/blob_info_v1.rs
Outdated
Show resolved
Hide resolved
crates/walrus-service/src/node/storage/blob_info/blob_info_v2.rs
Outdated
Show resolved
Hide resolved
crates/walrus-service/src/node/storage/blob_info/blob_info_v2.rs
Outdated
Show resolved
Hide resolved
crates/walrus-service/src/node/storage/blob_info/blob_info_v2.rs
Outdated
Show resolved
Hide resolved
crates/walrus-service/src/node/storage/blob_info/blob_info_v2.rs
Outdated
Show resolved
Hide resolved
crates/walrus-service/src/node/storage/blob_info/blob_info_v2.rs
Outdated
Show resolved
Hide resolved
…vel blob info module. This commit also include some minor code fixes
…ermanentBlobInfo, move PermanentBlobInfo to its own module
sadhansood
left a comment
There was a problem hiding this comment.
Thanks @halfprice, i did a pass but I do not have in depth expertise of this part of the codebase. I'll try to do another one later today.
crates/walrus-service/src/node/storage/blob_info/blob_info_v2.rs
Outdated
Show resolved
Hide resolved
halfprice
left a comment
There was a problem hiding this comment.
Thanks @sadhansood for the
crates/walrus-service/src/node/storage/blob_info/blob_info_v2.rs
Outdated
Show resolved
Hide resolved
|
Hi @mlegner , I think it'll be easier to review the comments commit by commit.
In short, what this PR introduces is only the aggregated BlobInfoV2. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 680d1abd18
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
680d1ab to
e0ca6c1
Compare
mlegner
left a comment
There was a problem hiding this comment.
Thanks a lot for the changes, @halfprice. This LGTM. 💯
Description
To easy review:
Contribute to WAL-1162
Contribute to WAL-1175
Test plan
Unit test
Release notes
Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.
For each box you select, include information after the relevant heading that describes the impact of your changes that
a user might notice and any actions they must take to implement updates. (Add release notes after the colon for each item)