Better error handling around missing ODR profiles #2756
Conversation
| @@ -166,7 +166,7 @@ func (s *service) queueJobReqToJob( | |||
| } | |||
| } | |||
|
|
|||
| // If the job can be run any any runner, then we attempt to see if we should spawn | |||
| // If the job can be run by any runner, then we attempt to see if we should spawn | |||
| // on on-demand runner for it. We only consider jobs for any runner because ones | |||
There was a problem hiding this comment.
| // on on-demand runner for it. We only consider jobs for any runner because ones | |
| // an on-demand runner for it. We only consider jobs for any runner because ones |
| if od == nil { | ||
| return nil, "", status.Error(codes.FailedPrecondition, "Missing required parameter OnDemandRunnerConfig") | ||
| } | ||
|
|
There was a problem hiding this comment.
Nit: I don't see any other way into this method outside of the above wrapJobWithRunner, which already performs a nil check on the OnDemandRunnerConfig, so this seems redundant.
There was a problem hiding this comment.
Came to say the same thing :)
|
In writing the tests, I wasn't able to recreate the problem exactly as described in this PR, so, since this isn't urgent and holidays mean we won't cross paths for a couple weeks, I'm just going to leave this little test here for @izaaklauer when he gets back. Take it or leave it 😄 I assume I am missing the crux of this edge case. https://github.com/hashicorp/waypoint/compare/odr-profile-not-found...copy-of-odr-profile-not-found?expand=1 |
|
Ahhh, oops. Looks like we didn't get to this one 😢 I accidentally redid it in #3054 and added a lil' test so I'm going to close this since they are the same! |
Catches an extremely corner-case panic.
Without this, if you try to upsert a project with a git ref, data polling disabled, and a runner profile ref that doesn't exist, we panic.