#95 spectool: return non-zero exit code if download fails (i.e. IOError)
Merged by ngompa. Opened by decathorpe.
Unknown source main

Download 95.patch

Exceptions thrown by requests (which are subclasses of IOError) are right now caught, but ignored. This results in a zero exit code, even if the download fails (c.f. https://bugzilla.redhat.com/show_bug.cgi?id=2096624 ).

This PR changes the download() function to re-raise any IOErrors that occur.

Additionally, the main() function is slightly refactored, to collect requested tasks and execute them in a loop. This way, any error handling (i.e. catching IOError and returning a non-zero exit code in that case) can happen in one place.

Not sure if the "early exit" is desirable (i.e. with this PR, spectool will abort after the first failed download), or if it should attempt to do all the things and just return a non-zero exit code if any of the requested downloads failed.

It'd probably be better to do the latter, don't you think? That way the user knows which ones don't work all at once.

rebased onto 7b60394e19243cd9b358a76af6d8f9eb52ee181f

Yeah, good point.

Meh, I just realized that I put the "did any of this fail" check in the wrong place. :(

rebased onto 352482586e4872ab174cb1dd5dc6d36eee578d31

Ok, spectool should now attempt to fetch everything that was requested (be that patches, sources, or both), print all errors that occur, and return with a non-zero exit code in case any request failed, and return 0 if they were all successful.

Keeping compatibility with how the "old" spectool worked is starting to result in kinda gnarly code :(

rebased onto 7b8aec77a7c12c25b3b6d5893a92061527c2db37

Pull-Request has been merged by ngompa

I built rpmdevtools from HEAD to test the new pyproject template and noticed that spectool -g now exit 1 when the sources are already downloaded, which breaks my package update script. I agree it should exit 1 if the download fails but returning an error code when the download was simply skipped seems wrong.

Uh, that's definitely not intentional ... at this point the code is pretty bad just to ensure compatibility with the original perl implementation of spectool, so it's possible this change unintentionally changed return code for "already exists" case :(

Metadata