Fix update-checkout error reporting #60234
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Member Author
| @swift-ci please test |
Member Author
| Pretty sure this one isn't me: |
Member Author
| @swift-ci please test macOS platform |
Member
| @swift-ci python lint |
In the event of a failure, update-checkout wouldn't always fail very gracefully. As a result the actual failure wasn't always printed, which makes it really difficult to figure out what's going on. In my case, I'm jumping between 5.6 and main branches, so the `swift-cmark-gfm` is causing update-checkout to hiccup since it's not in the `main` configuration file scheme for `main`, but is in the 5.6 repository. You wouldn't know that though, since it would complain about `repo_path` not being in the error message before crashing. I've gone ahead and done some cleanups, reporting what the problem is at the point of failure beyond the "key error 'swift-cmark-cfg'. Modernized the error reporter a bit, so it should be a smidge easier to read.
etcwilde force-pushed the ewilde/fix-update-checkout-error-reporting branch from f2c927b to 73d2ac1 Compare Member Author
| @swift-ci please smoke test |
Member Author
| @swift-ci please python lint |
porglezomp reviewed Aug 5, 2022
Member Author
| @swift-ci please smoke test |
Member Author
| @swift-ci python lint |
Removing the `exit(1)` after failing. This shouldn't have an impact since we've already failed and shouldn't have hit that. I may have kept it from earlier debugging. Replace `raise e` with just `raise`. It removes one level from the stack trace where the actual `raise` takes place. The place we're actually interested in is where the failure actually takes place.
etcwilde force-pushed the ewilde/fix-update-checkout-error-reporting branch from 8d80da8 to 71b3ff5 Compare Member Author
| @swift-ci please smoke test |
Member Author
| @swift-ci please python lint |
porglezomp approved these changes Aug 5, 2022
Comment on lines +74 to +78
| try: | ||
| raise r | ||
| except KeyError as e: | ||
| messages.append(f"Failed to look up {e} in scheme\n") | ||
| except Exception: |
Contributor
There was a problem hiding this comment.
I just realized you can make this a bit simpler and avoid the "raise for pattern matching" which is a bit quirky
Suggested change
| try: | |
| raise r | |
| except KeyError as e: | |
| messages.append(f"Failed to look up {e} in scheme\n") | |
| except Exception: | |
| if isinstance(r, KeyError): | |
| messages.append(f"Failed to look up {r} in scheme\n") | |
| else: |
Member Author
There was a problem hiding this comment.
I wasn't sure which read better. It should probably be single JobError with a __str__ method that just prints the right thing so this can just iterate over the list and print it without having to figure out what happened in the job that failed (even the KeyError here could lead to a misleading error message), but that is a bigger PR.
AnthonyLatsis added the update-checkout Member Author
| @swift-ci please test |
Member
| @etcwilde does the issue this fixes still exist? |
Member Author
| I believe the failure output is still not great. I should fix this back up and get it merged. |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
In the event of a failure, update-checkout wouldn't always fail very
gracefully. As a result the actual failure wasn't always printed, which
makes it really difficult to figure out what's going on.
In my case, I'm jumping between 5.6 and main branches, so
the
swift-cmark-gfmis causing update-checkout to hiccup since it'snot in the
mainconfiguration file scheme formain, but is in the5.6 repository. You wouldn't know that though, since it would complain
about
repo_pathnot being in the error message before crashing.I've gone ahead and done some cleanups, reporting what the problem is at
the point of failure beyond the "key error 'swift-cmark-cfg'.