Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw an error when the input version is not found #4740

Merged
merged 3 commits into from
Jan 2, 2020

Conversation

clarafu
Copy link
Contributor

@clarafu clarafu commented Nov 8, 2019

If we can't find a version for an input, we should throw an error while
we construct the plan.

Signed-off-by: Clara Fu cfu@pivotal.io

Contributor Checklist

Are the following items included as part of this PR? Please delete checkbox items that don't apply.

Reviewer Checklist

This section is intended for the core maintainers only, to track review progress. Please do not
fill out this section.

  • Code reviewed
  • Tests reviewed
  • Documentation reviewed
  • Release notes reviewed
  • PR acceptance performed
  • New config flags added? Ensure that they are added to the BOSH
    and Helm packaging; otherwise, ignored for the integration tests (for example, if they are Garden configs that are not displayed in the --help text).

@clarafu clarafu requested a review from a team November 8, 2019 17:29
Copy link
Member

@vito vito left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this works but I have no way of verifying it as we don't log or emit the error anywhere.

plan, err := s.factory.Create(job.Config(), resourceConfigs, resourceTypes, buildInputs)
if err != nil {
// Don't use ErrorBuild because it logs a build event, and this build hasn't started
if err = nextPendingBuild.Finish(db.BuildStatusErrored); err != nil {
logger.Error("failed-to-mark-build-as-errored", err)
}
return false, nil
}

Can we emit a log and show the error in the UI?

@clarafu
Copy link
Contributor Author

clarafu commented Nov 19, 2019

@vito Could we just log the error without showing it in the UI?

@vito
Copy link
Member

vito commented Nov 19, 2019

@clarafu no

@clarafu
Copy link
Contributor Author

clarafu commented Nov 19, 2019

@vito image

@deniseyu
Copy link
Contributor

Going to re-trigger testflight on this, it looks like a Docker flake

clarafu and others added 2 commits December 10, 2019 13:45
If we can't find a version for an input, we should throw an error while
we construct the plan.

Signed-off-by: Clara Fu <cfu@pivotal.io>
Signed-off-by: Clara Fu <cfu@pivotal.io>
Co-authored-by: Julia Pu <jpu@pivotal.io>
@clarafu clarafu force-pushed the fix-input-version-not-found branch from 837a09e to 225a1a1 Compare December 10, 2019 18:48
Copy link
Member

@vito vito left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to show it in the UI but this is good enough for now. Seems like showing it in the UI will take more work than expected.

@vito vito merged commit b21a57a into release/6.0.x Jan 2, 2020
@vito vito deleted the fix-input-version-not-found branch January 2, 2020 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants