Add json support for index.yaml file #10542#12151
Conversation
pull-request-size
Bot
added
the
size/L
|
@smmhatami please fix the DCO issue. maybe it's a breaking change? |
Signed-off-by: Seyed Mohammad Mahdi Hatami <[email protected]>
Signed-off-by: Seyed Mohammad Mahdi Hatami <[email protected]>
|
Changes LGTM |
| } | ||
|
|
||
| resp, err := r.Client.Get(indexURL, | ||
| resp, err := r.Client.Get(jsonIndexURL, |
There was a problem hiding this comment.
If the file does not exist (HTTP 404), this would return an error and it would not fallback to loading the index.yaml. See also
Lines 91 to 93 in 4e447d8
In addition, on a successful retrieval any parsing error would be swallowed by an attempt to fetch and load the index.yaml (which may not exist). Yielding absolutely no information to the user to figure out what went wrong.
Given this, I think it would be better to not perform a fallback based on loading errors. But to rather fall back based on the error from r.Client.Get, which may require introducing a typed error in httpgetter.go to distinguish a HTTP 404 from transient errors.
|
In addition, this lacks any configuration options to generate an |
Co-authored-by: Hidde Beydals <[email protected]> Signed-off-by: Seyed Mohammad Mahdi Hatami <[email protected]>
|
@smmhatami do you think you would be able to continue with this attempt based on my comments? Otherwise I would be happy to take it over, as I think it would be beneficial to Helm given the growing number of (bigger) indexes. |
|
Given the other PR has been merged, this can be closed. |
5 participants
This is a fix to solve Issue 10542. Resolves #10542. With these changes one can use json formatted index file instead of yaml. the code now looks for index.json file as default and switches to index.yaml if not found.