Skip to content

Separate CalledProcessError and FileNotFoundError exceptions #73

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

radeksm
Copy link

@radeksm radeksm commented Dec 15, 2024

Orginal logic does not make distinction between situation when:

  1. yq is missing, not installed, this problem is addressed here: deps: add yq as installation dependency instructlab#2796
  2. when yq found a problem with the format and validation command failed

This commit separate scenario [1] and [2]. Also improve warning messages, allows user to re-run the yq command and help find a problem within qna.yaml file.

Orginal logic does not make distinction between situation when:
 1. yq is missing, not installed, this problem is addressed here:
    instructlab/instructlab#2796
 2. when yq found a problem with the format and validation
    command failed

This commit separate scenario [1] and [2]. Also improve warning
messages, allows user to re-run the yq command and help find a problem
within qna.yaml file.

Signed-off-by: Radoslaw Smigielski <[email protected]>
Copy link
Contributor

@bjhargrave bjhargrave 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 any issue you see here is caused by using the incorrect yq command. See instructlab/instructlab#2796 (comment). If you use the yq python package, the line operator does not exist and you will get an error.

So a better idea might be to advise the user upon the operator error that they need to use the proper yq command, https://github.com/mikefarah/yq, and not the yq python package.

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.

2 participants