Fix update-checkout error reporting #60234
Conversation
|
@swift-ci please test |
|
Pretty sure this one isn't me: |
|
@swift-ci please test macOS platform |
|
@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
|
@swift-ci please smoke test |
|
@swift-ci please python lint |
|
@swift-ci please smoke test |
|
@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
|
@swift-ci please smoke test |
|
@swift-ci please python lint |
| try: | ||
| raise r | ||
| except KeyError as e: | ||
| messages.append(f"Failed to look up {e} in scheme\n") | ||
| except Exception: |
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
| 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: |
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
|
@swift-ci please test |
|
@etcwilde does the issue this fixes still exist? |
|
I believe the failure output is still not great. I should fix this back up and get it merged. |
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'.