Skip to content

feat: Add config-manager push restart command#97

Open
dallinjsevy wants to merge 3 commits into
mainfrom
feature/config-manager-push-restart
Open

feat: Add config-manager push restart command#97
dallinjsevy wants to merge 3 commits into
mainfrom
feature/config-manager-push-restart

Conversation

@dallinjsevy

Copy link
Copy Markdown

No description provided.

@dallinjsevy dallinjsevy force-pushed the feature/config-manager-push-restart branch from 5c40799 to 092fe7a Compare April 15, 2026 20:23

@phalestrivir phalestrivir left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

See comments below

Comment thread src/cli/config-manager/config-manager-push/config-manager-push-restart.ts Outdated
Comment thread src/cli/config-manager/config-manager-push/config-manager-push-restart.ts Outdated
Comment thread src/configManagerOps/FrConfigRestartOps.ts.ts
Comment thread src/configManagerOps/FrConfigRestartOps.ts.ts
Comment thread src/configManagerOps/FRConfigRestart.ts Outdated
Comment thread src/configManagerOps/FRConfigRestart.ts Outdated
Comment thread src/configManagerOps/FRConfigRestart.ts Outdated
Comment thread src/configManagerOps/FRConfigRestart.ts Outdated
Comment thread src/cli/config-manager/config-manager-push/config-manager-push-restart.ts Outdated
@dallinjsevy dallinjsevy force-pushed the feature/config-manager-push-restart branch from 4af46ce to b2cca19 Compare April 22, 2026 17:15

@phalestrivir phalestrivir left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Make sure to run npm run lint:fix when you are finished with these last changes, I noticed a few formatting issues that would've been fixed with it.

Comment on lines +41 to +50
if (await getTokens(false, true, deploymentTypes)) {
verboseMessage('Restarting Tenant');
const outcome = await configManagerRestart(

options.status,
options.check,
options.wait
);
if (!outcome) process.exitCode = 1;
}});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Change this code to look something like this:

const getTokensIsSuccessful = await getTokens(
false,
true,
deploymentTypes
);
if (!getTokensIsSuccessful) process.exit(1);
verboseMessage('Importing email provider configuration.');
const outcome = await configManagerImportEmailProvider();
if (!outcome) process.exitCode = 1;

What you have is almost correct, but I noticed you removed the process.exitCode=1 in the event that getTokens fails. If getTokens fails, we need to exit with code 1, which this example shows

printMessage(await readStatus());
return true;
}
if (check) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Right before this if statement, we should check if the status is "restarting" since fr config manager does this:

if (restartStatus === "restarting") {
  printMessage("Environment already restarting.", "error");
  return false;
}

Before the if statement for status, I would do

const restartStatus = await readStatus();

And use that result in the if statement for status. That way we can reuse the status for this new if statement.

Comment on lines +31 to +32
const outcome = await applyUpdates(!!wait);
return outcome;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Simplify this to

return await applyUpdates(!!wait);

Comment on lines +71 to +72
const { stdout } = await exec(CMD, cloudEnv);
expect(removeAnsiEscapeCodes(stdout)).toMatchSnapshot();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is more of a suggestion, but since you need to make other changes I thought I'd mention it.

I created a helper that simplifies this to:

await testSuccess(CMD, cloudEnv);

I would update your tests to use it just to help simplify the testing code a bit.

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