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.

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

Run isRunning and isEnabled as sudo #672

Merged
merged 2 commits into from
Mar 20, 2018

Conversation

aileen
Copy link
Member

@aileen aileen commented Mar 16, 2018

closes #660
refs #414

Recently there were many users reporting issues when the CLI is running the command systemctl is-active. With the changes in #669 we are now able to run the isRunning and isEnabled methods in our systemd extension, as those were the only commands that weren't executed as sudo.

Manual tests worked fine for me. I can't say if it'll fix all the issues, as I wasn't able to reproduce the issues on the recommended stack. Nevertheless, there were enough reports of this issue actually happening on the recommended stack.

@coveralls
Copy link

coveralls commented Mar 16, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling df0bce1 on AileenCGN:sudo-systemd-isactive into c93124b on TryGhost:master.

Copy link
Member

@vikaspotluri123 vikaspotluri123 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@acburdine
Copy link
Member

acburdine commented Mar 17, 2018

So the isRunning part looks good, however the isEnabled part won't work quite right. When I converted the isRunning stuff to handle promises, I didn't convert the isEnabled code to handle them as well (see here and here). The reason this PR doesn't break anything is that the Promise returned by the isEnabled function technically is a truthy value, so the if statement in both those commands passes 🙈

I'll work on updating the isEnabled code as well, which should make this PR work as expected, but wanted to comment here first so it doesn't get merged before that's been done 😉

Edit: #674 has the other changes required for this PR to work.

acburdine added a commit to acburdine/Ghost-CLI that referenced this pull request Mar 18, 2018
refs TryGhost#672
- support isEnabled returning a promise
aileen pushed a commit that referenced this pull request Mar 19, 2018
Copy link
Member

@acburdine acburdine left a comment

Choose a reason for hiding this comment

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

Tested this locally, works great :) thanks!

@acburdine acburdine merged commit 5552a0e into TryGhost:master Mar 20, 2018
@aileen aileen deleted the sudo-systemd-isactive branch May 10, 2018 01:50
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.

Ghost CLI Validating config undefined.
4 participants