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

Change output of errors for native ssh (#1012) #1051

Merged
merged 2 commits into from
Feb 25, 2017

Conversation

pluseg
Copy link
Contributor

@pluseg pluseg commented Feb 24, 2017

Q A
Bug fix? No
New feature? Yes
BC breaks? No
Deprecations? No
Fixed tickets #1012

@@ -18,8 +20,6 @@
- Fixed possibility to use PEM files with Native SSH
- Fixed old releases not being cleaned up when keep_releases reduced by more than half.

### Changed
- Add task queue:restart for Laravel recipe [#1007](https://github.com/deployphp/deployer/pull/1007)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just combined with same section above.

@@ -49,7 +50,6 @@ public function run($command)
$serverConfig = $this->getConfiguration();
$sshOptions = [
'-A',
'-q',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Elfet Is there a real reason of adding "-q" option? Without it ssh returns 255 status code (Unknown error) and empty output.

Copy link
Member

Choose a reason for hiding this comment

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

Don't know 🙄 It came from PR.

->setIdleTimeout(null)
->mustRun();
} catch (ProcessFailedException $exception) {
$errorMessage = \Deployer\isVerbose() ? $exception->getMessage() : $process->getErrorOutput();
Copy link
Member

Choose a reason for hiding this comment

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

Why in verbose mode using message and otherwise error output?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Elfet Error output returns only error string like "invalid credentials" and an exception message has current look with full broken command, current directory, error string, etc. I think that common user needs only concise error string.

Copy link
Member

Choose a reason for hiding this comment

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

But why not isDebug then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Elfet I used it but then noted that there is one usage of Verbose in the file. Okay, I'll update the PR.

@antonmedv antonmedv closed this Feb 24, 2017
@antonmedv antonmedv reopened this Feb 24, 2017
@antonmedv antonmedv merged commit fa05a68 into deployphp:master Feb 25, 2017
@pluseg pluseg deleted the 1012-native-ssh-errors branch February 25, 2017 04:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants