cnabio / cnab-go

A Go implementation of CNAB Core 1.0
MIT License
69 stars 35 forks source link

fix: avoid erasing local schema if fetching schema from cdn.cnab.io fails #290

Closed epsxy closed 2 years ago

epsxy commented 2 years ago

Motivation

Fixes #287

Everything is detailled in this comment, but, TLDR: The CI is using the fetch-schemas go program to download the latest schema data files from the cnab cdn. If it doesn't reply for some reason, the program is erasing the local version of the schema file by an empty file, which can cause 2 tests to fail:

This PR is adding a return when the fetch request fails, so that we don't write an empty file on the disk when it fails and so that the tests don't fail in that case.

Reproducing the issue

The issue can be reproduced by running locally go run fetch-schemas.go. If you run this command before this PR, after turning off your internet connection (eg wifi), you will see that both files gets erased. After the PR, the files are not updated anymore (with an empty content) when you run this command without an internet connection.

Discussion

There's still a possibility that we don't want that PR, especially in the case that we want to be conservative and fail if we don't have the exact version of the spec we need (especially if we don't think the version in the repo is up to date). In that case maybe we would like the tests to fail rather than maybe releasing a false negative (if the version in the repo is not the same than the version upstream on the CDN).