-
Notifications
You must be signed in to change notification settings - Fork 44
[bundles] Add range request support to other support bundle APIs #8202
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
Conversation
@@ -7088,7 +7088,10 @@ impl NexusExternalApi for NexusExternalApiImpl { | |||
crate::context::op_context_for_external_api(&rqctx).await?; | |||
|
|||
let head = false; | |||
let range = rqctx.range(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As you can see here, we actually were parsing the range request headers before - just not visibly through the API spec.
@@ -290,7 +290,10 @@ impl SledAgentApi for SledAgentImpl { | |||
file, | |||
} = path_params.into_inner(); | |||
|
|||
let range = rqctx.range(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as for the Nexus APIs - the sled agent already knew how to process these range requests, it just wasn't part of the openapi spec.
@@ -123,6 +123,7 @@ impl super::Nexus { | |||
&ZpoolUuid::from(bundle.zpool_id), | |||
&DatasetUuid::from(bundle.dataset_id), | |||
&SupportBundleUuid::from(bundle.id), | |||
range, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a minor change, but this part is actually critical: for all these APIs, where Nexus was proxying requests to the sled agent over progenitor APIs, it was previously unable to forward range request headers.
Now, with the new progenitor work, it can do so, for all bundle-related APIs.
(This is wiring up the same thing we did already in #8183 , but for other more granular bundle access)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very straightforward, nice job laying the groundwork for this last year.
#8183 plumbed range request support through Nexus for the "download entire support bundle" API.
This PR continues in that direction, plumbing range request support for all other "download portions of the bundle" APIs.