Skip to content

Allow TNetXNGFile to use the http protocol#22025

Open
ffurano wants to merge 1 commit intoroot-project:masterfrom
ffurano:fab-xrdcl-allow-http
Open

Allow TNetXNGFile to use the http protocol#22025
ffurano wants to merge 1 commit intoroot-project:masterfrom
ffurano:fab-xrdcl-allow-http

Conversation

@ffurano
Copy link
Copy Markdown

@ffurano ffurano commented Apr 23, 2026

TNetXNGFile was not working correctly when given http URLs. This PR fixes that and unlocks progress on those http/s3 matters

@ffurano ffurano requested a review from dpiparo as a code owner April 23, 2026 09:33
Copy link
Copy Markdown
Contributor

@jblomer jblomer left a comment

Choose a reason for hiding this comment

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

Thank you for the patch!

If I understand correctly, this will make XRootD the handler for HTTP(S) instead of DAVIX. This is a significant change which, I think, requires more testing. E.g., what happens if a version of XRootD is linked that does not support HTTP. Also, it may need a fallback (for some time) in the same way that Davix had a fall back to the old TWebFile.

What about the dav(s) and s3(s) protocol identifiers: should they be deprecated (the PR suggests that).

@jblomer jblomer self-assigned this Apr 23, 2026
@jblomer jblomer added in:I/O and removed in:I/O labels Apr 23, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 23, 2026

Test Results

    22 files      22 suites   3d 7h 5m 31s ⏱️
 3 850 tests  3 847 ✅  1 💤 2 ❌
76 007 runs  75 985 ✅ 18 💤 4 ❌

For more details on these failures, see this check.

Results for commit d0bd605.

♻️ This comment has been updated with latest results.

Comment thread etc/plugins/TFile/P100_TXNetFile.C Outdated
Comment thread etc/plugins/TFile/P130_TDavixFile.C Outdated
Comment thread net/netxng/src/TNetXNGFile.cxx
@ffurano
Copy link
Copy Markdown
Author

ffurano commented Apr 23, 2026

Hi Jakob,

what you mention will be among the next steps, and it will take longer. I did not intend to change the default config of the root plugin manager, so XrdCl shall not substitute Davix by default.

The 1-line code change removes the wrong bit in root that was preventing XrdCl from being invoked with non-xroot protocols. For some reason the change in the plugin manager slipped into. My mistake. I'll redo it.

Thank you for the patch!

If I understand correctly, this will make XRootD the handler for HTTP(S) instead of DAVIX. This is a significant change which, I think, requires more testing. E.g., what happens if a version of XRootD is linked that does not support HTTP. Also, it may need a fallback (for some time) in the same way that Davix had a fall back to the old TWebFile.

What about the dav(s) and s3(s) protocol identifiers: should they be deprecated (the PR suggests that).

@ffurano ffurano closed this Apr 23, 2026
@ffurano ffurano reopened this Apr 23, 2026
@ffurano ffurano force-pushed the fab-xrdcl-allow-http branch from 7faf22f to d0bd605 Compare April 23, 2026 14:02
@ffurano ffurano requested a review from jblomer April 23, 2026 14:10
@amadio
Copy link
Copy Markdown
Member

amadio commented Apr 23, 2026

Looks good now. I will try this with https://github.com/root-project/analysis-grand-challenge over HTTP and post some results here later.

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.

3 participants