Skip to content

CFE-4661: Fixed an issue where wrong names in --clients would cause cf-remote to hang#186

Open
sarakthon wants to merge 2 commits into
cfengine:masterfrom
sarakthon:check-name
Open

CFE-4661: Fixed an issue where wrong names in --clients would cause cf-remote to hang#186
sarakthon wants to merge 2 commits into
cfengine:masterfrom
sarakthon:check-name

Conversation

@sarakthon

@sarakthon sarakthon commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Command and output before changes:
Testing cf-remote install with wrong/undefined name in --clients:

% cf-remote install --clients test

The command gives no output, and the program hangs for an indefinite time period.

Command and output after changes:

% cf-remote install --clients test

Error: 'test' does not exist.

@sarakthon sarakthon changed the title Check name CFE-4661: Check name Jun 5, 2026
@olehermanse olehermanse changed the title CFE-4661: Check name CFE-4661: Fixed an issue where wrong names in --clients would cause cf-remote to hang Jun 5, 2026

@olehermanse olehermanse left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's do a f2f review on this one

@sarakthon sarakthon force-pushed the check-name branch 3 times, most recently from c56bfed to e625027 Compare June 10, 2026 14:31

@olehermanse olehermanse left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add a comment in this PR showing what you have tested (commands and output), before and after your changes.

@sarakthon sarakthon requested a review from olehermanse June 12, 2026 12:53

@olehermanse olehermanse left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for adding the test / command output. Please expand it to cover the other cases we looked at (group which exists and works, before and after, with and without @ before group name. Non-existing group preceded by @ and localhost).

@olehermanse

Copy link
Copy Markdown
Member

Test failure is unrelated and should be fixed by #189

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants