Skip to content

cli: try command - handle insufficient user privilege error gracefully - #611

Merged
JegadishD merged 1 commit into
canonical:mainfrom
JegadishD:jegadish/try_cmd_fix
Sep 3, 2026
Merged

JegadishD merged 1 commit into
canonical:mainfrom
JegadishD:jegadish/try_cmd_fix

Conversation

@JegadishD

Copy link
Copy Markdown
Collaborator

Insufficient privileges is leading to exception and the following revert as well fails with exception which is not handled and triggers apport crash reports. The fix is to catch on revert and exit cleanly with error messages.

@JegadishD
JegadishD marked this pull request as ready for review August 31, 2026 12:21
@JegadishD
JegadishD requested a review from benhoyt August 31, 2026 12:21

@benhoyt benhoyt left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The overall approach looks reasonable, but a couple of minor comments for consideration.

Also, I'd retitle the PR: the fix here is to handle all errors during revert, not just insufficient user privilege. You should of course mention the specific case in the PR description, though.

Comment thread netplan_cli/cli/commands/try_command.py
Comment thread netplan_cli/cli/commands/try_command.py Outdated
@JegadishD
JegadishD force-pushed the jegadish/try_cmd_fix branch from 471d38b to ce01396 Compare September 2, 2026 06:51
For example, when `netplan try` is run without sufficient privileges,
it throws an exception. The subsequent revert operation also fails,
and this unhandled exception triggers apport crash reports.
@JegadishD
JegadishD force-pushed the jegadish/try_cmd_fix branch from ce01396 to c7f1823 Compare September 2, 2026 06:57
@JegadishD

Copy link
Copy Markdown
Collaborator Author

@benhoyt Thanks for the review. I have updated the PR based on the review comments. Please review.

@JegadishD
JegadishD requested a review from benhoyt September 2, 2026 09:35
@benhoyt

benhoyt commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Thanks, looks good to me. I'm assuming you've done some manual testing locally as well.

@JegadishD
JegadishD merged commit 464bc27 into canonical:main Sep 3, 2026
17 checks passed
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