Skip to content
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

schema/cosa/build: check http response codes in http.Get #2989

Merged
merged 1 commit into from
Jul 18, 2022

Conversation

dustymabe
Copy link
Member

This will make the error more clear when trying to run
an upgrade test where the fedora-coreos.parent-version`
is not actually a real version in the stream history.

I'm running a separate pipeline right now and so I'm
pushing dev builds into s3 and my parent version is
36.20220716.dev.1. That doesn't exist in the real
testing-devel stream so the code barfs like this:

$ cosa kola --rerun --upgrades --no-test-exit-error
kola -p qemu-unpriv --output-dir tmp/kola-upgrade run-upgrade --rerun --no-test-exit-error --qemu-image-dir tmp/kola-qemu-cache -v --find-parent-image
2022-07-16T21:52:14Z cli: Started logging at level INFO
2022-07-16T21:52:14Z cli: Started logging at level INFO
Error: failed to parse build: invalid character '<' looking for beginning of value

This will improve that error message.

@@ -305,6 +305,10 @@ func FetchAndParseBuild(url string) (*Build, error) {
return nil, err
}
defer res.Body.Close()
if res.StatusCode >= 300 {
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be e.g.

Suggested change
if res.StatusCode >= 300 {
if res.StatusCode != 200 {

instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not super famililiar with http status codes. I thought everything in the 200s was successful, but maybe for our particular case here they don't apply?

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed!

This will make the error more clear when trying to run
an upgrade test where the fedora-coreos.parent-version`
is not actually a real version in the stream history.

I'm running a separate pipeline right now and so I'm
pushing dev builds into s3 and my parent version is
`36.20220716.dev.1`. That doesn't exist in the real
testing-devel stream so the code barfs like this:

```
$ cosa kola --rerun --upgrades --no-test-exit-error
kola -p qemu-unpriv --output-dir tmp/kola-upgrade run-upgrade --rerun --no-test-exit-error --qemu-image-dir tmp/kola-qemu-cache -v --find-parent-image
2022-07-16T21:52:14Z cli: Started logging at level INFO
2022-07-16T21:52:14Z cli: Started logging at level INFO
Error: failed to parse build: invalid character '<' looking for beginning of value
```

This will improve that error message.
@dustymabe dustymabe merged commit cb65eb3 into coreos:main Jul 18, 2022
@dustymabe dustymabe deleted the dusty-fix-error-message branch July 18, 2022 21:50
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.

None yet

2 participants