Update validators.py #703
Update validators.py #703
Conversation
| @@ -148,7 +148,7 @@ def validate_build_tags(request): | |||
| if not build_rel: | |||
| raise KeyError("Couldn't find release from build tags") | |||
| except KeyError: | |||
| msg = 'Cannot find release associated with build: {}, tags: {}'.format(build, tags) | |||
| msg = 'Cannot find any tags associated with that nvr: {}, tags: {}'.format(build, tags) | |||
There was a problem hiding this comment.
It would be nice to have the error messages be different. Maybe this could read something like
'nvr for {} is not tagged with any of the following: {}'.format(build, tags)
There was a problem hiding this comment.
tags in this context are what the build is tagged with, so that message wouldn't be correct.
Also, I'm not sure that nvr is the most user-friendly way to describe a build, even though most people probably know what it means.
There was a problem hiding this comment.
Fair enough. The same sentiment still remains though. I think having different error messages would be nicer than a single error message for both lines.
There was a problem hiding this comment.
different error massages are updated to issue #630
different error massages are updated to issue #630
|
OK. There's really two failure scenarios here.
Seems like we should add a check before the call to if not tags:
# return some error here indicating that the build has *no* tagsand then the error messages that have already been edited here need to be updated to indicate that we have hit the second scenario. FWIW, I agree with the comment on an earlier commit that we should replace the word 'nvr' in the error messages with 'build'. |
|
Also, @TridevGuha, did you run the test suite against this change? I have a feeling that there are test cases that check these error messages (although I'm not sure about that). |
|
Nope, I did not run the test suite, need to run the test? |
|
Yeah, definitely. They're pretty comprehensive. |
|
@TridevGuha any luck running the test suite? |
|
OK, I'm going to close this one for now. Feel free to re-open it after we can get some feedback from the test suite. |
#630