Skip to content

[ntuple] Encapsulate RNTupleAnchorS3 fields - #22952

Merged
silverweed merged 1 commit into
root-project:masterfrom
JasMehta08:s3-anchor-encapsulation
Jul 30, 2026
Merged

[ntuple] Encapsulate RNTupleAnchorS3 fields#22952
silverweed merged 1 commit into
root-project:masterfrom
JasMehta08:s3-anchor-encapsulation

Conversation

@JasMehta08

Copy link
Copy Markdown
Contributor

This Pull request:

(Is a part of the GSoC 2026 project S3 Backend for RNTuple.)

Changes or fixes:

  • Change RNTupleAnchorS3 from a struct with public fields to a class with private fields and public getters.
  • Declare RPageSinkS3 as a friend so the write path can set fields directly.
  • Update tests to construct anchors via CreateFromJSON using test-local helpers (MakeAnchorJson, MakeAnchor) and read fields through getters.
  • Link nlohmann_json and xxHash to the S3 test target for anchor construction and checksum computation in tests.

Checklist:

  • tested changes locally

This PR is a follow up of #22819

Comment thread tree/ntuple/inc/ROOT/RPageStorageS3.hxx Outdated
Comment thread tree/ntuple/test/ntuple_storage_s3.cxx Outdated
Comment thread tree/ntuple/test/ntuple_storage_s3.cxx Outdated
@JasMehta08
JasMehta08 force-pushed the s3-anchor-encapsulation branch 2 times, most recently from 5a847f7 to 7bc062b Compare July 29, 2026 13:23

@silverweed silverweed left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@github-actions

github-actions Bot commented Jul 29, 2026

Copy link
Copy Markdown

Test Results

    23 files      23 suites   3d 18h 3m 33s ⏱️
 3 882 tests  3 882 ✅ 0 💤 0 ❌
79 059 runs  79 059 ✅ 0 💤 0 ❌

Results for commit 7db692a.

♻️ This comment has been updated with latest results.

@JasMehta08
JasMehta08 force-pushed the s3-anchor-encapsulation branch from 7bc062b to 7db692a Compare July 30, 2026 05:13
@JasMehta08

Copy link
Copy Markdown
Contributor Author

@silverweed, all the tests were passing, but there was a small error with the clang format, I have updated the code, I think now that should also pass as well

@silverweed
silverweed merged commit 62e8e57 into root-project:master Jul 30, 2026
34 checks passed
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