This is an archive of the discontinued Mercurial Phabricator instance.

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

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

Details

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
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

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
311

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

Same for this type of change

This revision now requires changes to proceed.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–209

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
This revision was automatically updated to reflect the committed changes.