This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1431

sshpeer: add a configurable hint for the ssh error message
ClosedPublic

Authored by zuza on Nov 15 2017, 7:43 PM.

Details

Reviewers
durham
mitrandir
durin42
Group Reviewers
hg-reviewers
Commits
rHG8b1c887d52e7: sshpeer: add a configurable hint for the ssh error message
Summary

Adding a possibility to configure error hint to be shown in the case of problems with SSH. Example of such hint can be "Please see http://company/internalwiki/ssh.html".

Test Plan
  • Ran hg pull with broken link and verified the output has no hint by default:
pulling from ssh://brokenrepository.com//repo
remote: ssh: Could not resolve hostname brokenrepository.com: Name or service not known
abort: no suitable response from remote hg!
pulling from ssh://brokenrepository.com//repo
remote: ssh: Could not resolve hostname brokenrepository.com: Name or service not known
abort: no suitable response from remote hg!
(Please see http://company/internalwiki/ssh.html)

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

zuza created this revision.Nov 15 2017, 7:43 PM
zuza edited the summary of this revision. (Show Details)Nov 15 2017, 7:51 PM
zuza edited the test plan for this revision. (Show Details)
zuza added reviewers: durham, mitrandir.
zuza updated this revision to Diff 3540.Nov 15 2017, 7:58 PM
zuza edited the test plan for this revision. (Show Details)Nov 15 2017, 8:48 PM
zuza updated this revision to Diff 3541.
zuza updated this revision to Diff 3542.Nov 15 2017, 8:52 PM
durham requested changes to this revision.Nov 15 2017, 9:05 PM
durham added inline comments.
mercurial/sshpeer.py
210

The standard Mercurial way to format this would be something like:

msg = self.ui.config("ssh", "errormessage",
                     _("no suitable response from remote hg"))
self._abort(error.RepoError(msg))
tests/test-ssh.t
314 ↗(On Diff #3542)

These lines seem unrelated to your change and therefore probably shouldn't have changed. Usually this is caused by your editor trying to remove trailing whitespace from a file, then run-tests -i adding the missing new line back in. Can you undo these new line changes? 'hg uncommit tests/test-ssh.t && hg commit -i' could make it pretty straightforward.

447 ↗(On Diff #3542)

Same for this type of change

This revision now requires changes to proceed.Nov 15 2017, 9:05 PM
durham added a comment.Nov 15 2017, 9:05 PM

I'd also update the commit summary with an example of what a better message might be. Like "there was an ssh error, please see http://company/internalwiki/ssh.html"

zuza updated this revision to Diff 3548.Nov 15 2017, 9:18 PM
zuza marked 3 inline comments as done.Nov 15 2017, 9:19 PM
yuja added a subscriber: yuja.Nov 16 2017, 8:07 AM
In D1431#23709, @durham wrote:

I'd also update the commit summary with an example of what a better message might be. Like "there was an ssh error, please see http://company/internalwiki/ssh.html"

So the purpose of this change is quite similar to ui.supportcontact, right?

My two cents. I think it's probably better to make an additional hint configurable,
instead of replacing the original message.

mercurial/sshpeer.py
207–211

new config needs to be registered to the configitems module.

zuza edited the summary of this revision. (Show Details)Nov 16 2017, 1:58 PM
zuza edited the test plan for this revision. (Show Details)
zuza retitled this revision from sshpeer: making the ssh error message configurable to sshpeer: add a configurable hint for the ssh error message.
zuza updated this revision to Diff 3572.
zuza marked an inline comment as done.Nov 16 2017, 1:59 PM
durin42 accepted this revision.Nov 17 2017, 4:03 PM
durin42 requested changes to this revision.Nov 17 2017, 5:06 PM

D'oh: this also needs to be documented, probably in hg help config. That's from mercurial/help/config.txt - hopefully it'll be fairly straightforward to add?

This revision now requires changes to proceed.Nov 17 2017, 5:06 PM
zuza updated this revision to Diff 3631.Nov 20 2017, 4:44 AM
durin42 accepted this revision.Nov 20 2017, 6:29 PM
Closed by commit rHG8b1c887d52e7: sshpeer: add a configurable hint for the ssh error message (authored by zuza). · Explain WhyNov 20 2017, 6:36 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/sshpeer.py (7 lines)

Diff 3539

mercurial/sshpeer.py

sub = util.popen4(cmd, bufsize=0) sub = util.popen4(cmd, bufsize=0)
self._pipeo, self._pipei, self._pipee, self._subprocess = sub self._pipeo, self._pipei, self._pipee, self._subprocess = sub
self._pipei = util.bufferedinputpipe(self._pipei) self._pipei = util.bufferedinputpipe(self._pipei)
self._pipei = doublepipe(self.ui, self._pipei, self._pipee) self._pipei = doublepipe(self.ui, self._pipei, self._pipee)
self._pipeo = doublepipe(self.ui, self._pipeo, self._pipee) self._pipeo = doublepipe(self.ui, self._pipeo, self._pipee)
def badresponse(): def badresponse():
self._abort(error.RepoError(_('no suitable response from '
'remote hg'))) self._abort(error.RepoError(self.ui.config(
"ssh",
"errormessage",
durhamUnsubmitted
Done

The standard Mercurial way to format this would be something like:

msg = self.ui.config("ssh", "errormessage",
                     _("no suitable response from remote hg"))
self._abort(error.RepoError(msg))
durham: The standard Mercurial way to format this would be something like: ``` msg = self.ui.config…
_("no suitable response from remote hg"))))
yujaUnsubmitted
Done

new config needs to be registered to the configitems module.

yuja: new config needs to be registered to the configitems module.
try: try:
# skip any noise generated by remote shell # skip any noise generated by remote shell
self._callstream("hello") self._callstream("hello")
r = self._callstream("between", pairs=("%s-%s" % ("0"*40, "0"*40))) r = self._callstream("between", pairs=("%s-%s" % ("0"*40, "0"*40)))
except IOError: except IOError:
badresponse() badresponse()