[Registry] Make 'Content-Version' optional for archive and manifest download API #6153
Conversation
yim-lee
requested review from
abertelrud,
neonichu,
tomerd and
elsh
as code owners
|
@swift-ci please test |
|
@swift-ci please smoke test |
| @@ -1379,7 +1391,7 @@ paths: | |||
| schema: | |||
| type: integer | |||
| Content-Version: | |||
| $ref: "#/components/headers/contentVersion" | |||
| $ref: "#/components/headers/optionalContentVersion" | |||
There was a problem hiding this comment.
AFAIK we can't override $ref.
Ideally we'd want to do something like this:
| $ref: "#/components/headers/optionalContentVersion" | |
| $ref: "#/components/headers/contentVersion" | |
| required: false |
There was a problem hiding this comment.
That's right, whether a header is required is part of the content, so to have an optional variant of the same header, we have to fully duplicate it.
|
@swift-ci please smoke test Windows |
| fileprivate func validateAPIVersion(_ expectedVersion: RegistryClient.APIVersion = .v1) throws { | ||
| guard self.apiVersion == expectedVersion else { | ||
| fileprivate func validateAPIVersion(_ expectedVersion: RegistryClient.APIVersion = .v1, isOptional: Bool = false) throws { | ||
| let apiVersion = self.apiVersion |
There was a problem hiding this comment.
We use this so we don't need to call self.apiVersion twice.
|
@swift-ci please smoke test macOS |
Documentation/Registry.md
Outdated
| schema: | ||
| type: string | ||
| enum: | ||
| - - "1" |
There was a problem hiding this comment.
Why the double dash here? (Both here and in contentVersion above), I'd expect just
enum:
- "1"There was a problem hiding this comment.
I am not sure. I thought it was some weird syntax that I didn't know about. Amended in 69a4a02
yim-lee
force-pushed
the
optional-api-version
branch
from
e0021d8 to
69a4a02
Compare
|
@swift-ci please smoke test |
|
@swift-ci please smoke test Windows |
…ownload API Motivation: The 'Content-Version' header is required in all registry server responses to indicate API version. However, it shouldn't be required for responses that don't/can't change, such as the download archive and fetch package manifest endpoints. This may also cause problems for registry that have these files hosted elsewhere, because they may not have full control over response headers. Modifications: - Modify registry API spec to make 'Content-Version' optional for the said endpoints - Adjust `RegistryClient` rdar://105415468
yim-lee
force-pushed
the
optional-api-version
branch
from
69a4a02 to
6928903
Compare
|
@swift-ci please smoke test |
|
@swift-ci please smoke test Windows |
Motivation:
The 'Content-Version' header is required in all registry server responses to indicate API version. However, it shouldn't be required for responses that don't/can't change, such as the download archive and fetch package manifest endpoints. This may also cause problems for registry that have these files hosted elsewhere, because they may not have full control over response headers.
Modifications:
RegistryClientrdar://105415468