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.

Sign up for GitHub

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

Jump to bottom

fix: Log brief msg instead of Traceback when cmd not found #3628

Merged
merged 0 commits into from
Dec 14, 2022
Merged

fix: Log brief msg instead of Traceback when cmd not found #3628

merged 0 commits into from
Dec 14, 2022

Conversation

xiangce
Copy link
Contributor

@xiangce xiangce commented Dec 13, 2022 •
edited
Loading

All Pull Requests:

Check all that apply:

  • Have you followed the guidelines in our Contributing document, including the instructions about commit messages?
  • Is this PR to correct an issue?
  • Is this PR an enhancement?

Complete Description of Additions/Changes:

@xiangce xiangce requested a review from ryan-blakley December 13, 2022 12:35
@xiangce
Copy link
Contributor Author

xiangce commented Dec 13, 2022

Please hold on reviewing this PR, I think we can just avoid raising the exceptions in the log but keep them in the meta_data

@xiangce xiangce marked this pull request as draft December 13, 2022 13:10
@xiangce xiangce removed the request for review from ryan-blakley December 13, 2022 13:11
@xiangce xiangce force-pushed the fix_3584 branch 2 times, most recently from 73ee1d7 to 080045c Compare December 13, 2022 14:18
@xiangce xiangce changed the title fix: Skip it instead of ContentException when no such command/file fix: Log the brief message instead of Traceback when no such cmd Dec 13, 2022
@xiangce xiangce changed the title fix: Log the brief message instead of Traceback when no such cmd fix: Log brief msg instead of Traceback when cmd not found Dec 13, 2022
@xiangce xiangce requested a review from ryan-blakley December 13, 2022 14:24
@xiangce xiangce marked this pull request as ready for review December 13, 2022 14:24
@xiangce
Copy link
Contributor Author

xiangce commented Dec 13, 2022

@ryan-blakley, hi, this MR is ready now, please kindly review when free.

ryan-blakley
ryan-blakley approved these changes Dec 13, 2022
View reviewed changes
Copy link
Contributor

@ryan-blakley ryan-blakley left a comment

Choose a reason for hiding this comment

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

This looks like a good change, it definitely makes it clearer that the base command isn't found instead of stating it couldn't execute the whole command.

@xiangce xiangce merged commit 1e19ef0 into master Dec 14, 2022
@xiangce xiangce deleted the fix_3584 branch December 14, 2022 01:01
xiangce added a commit that referenced this pull request Dec 14, 2022
@xiangce
fix: Log brief msg instead of Traceback when cmd not found (#3628)
1652f8f 
Signed-off-by: Xiangce Liu <[email protected]>
(cherry picked from commit 1e19ef0)
@xiangce xiangce mentioned this pull request Dec 14, 2022
Closed
xiangce added a commit that referenced this pull request Sep 6, 2024
@xiangce
fix: Log brief msg instead of Traceback when cmd not found (#3628)
21295cd 
Signed-off-by: Xiangce Liu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ryan-blakley ryan-blakley ryan-blakley approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Unexpected Traceback messages of "luksmeta" spec?
2 participants
@xiangce @ryan-blakley