[PATCH 03 of 11] obsolete: raise richer exception on unknown version

[Pierre-Yves David]

On 05/28/2017 08:56 PM, Gregory Szorc wrote:
> On Sun, May 28, 2017 at 9:32 AM, Pierre-Yves David
> <pierre-yves.david at ens-lyon.org <mailto:pierre-yves.david at ens-lyon.org>>
> wrote:
>
>     # HG changeset patch
>     # User Pierre-Yves David <pierre-yves.david at octobus.net
>     <mailto:pierre-yves.david at octobus.net>>
>     # Date 1495723823 -7200
>     #      Thu May 25 16:50:23 2017 +0200
>     # Node ID b97c492388f6b14c346600e1c1ed4e400f6b47ca
>     # Parent  dbce9b63b68d45706d9c269543070e0eee88b376
>     # EXP-Topic obsstrip
>     # Available At
>     https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
>     <https://www.mercurial-scm.org/repo/users/marmoute/mercurial/>
>     #              hg pull
>     https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
>     <https://www.mercurial-scm.org/repo/users/marmoute/mercurial/> -r
>     b97c492388f6
>     obsolete: raise richer exception on unknown version
>
>     We raise a more precise subclass of Abort with details about the faulty
>     version.  This will be used to detect this case  and display some
>     information
>     in debugbundle.
>
>     diff --git a/mercurial/error.py b/mercurial/error.py
>     --- a/mercurial/error.py
>     +++ b/mercurial/error.py
>     @@ -138,6 +138,14 @@ class UnsupportedMergeRecords(Abort):
>                  hint=_('see
>     https://mercurial-scm.org/wiki/MergeStateRecords
>     <https://mercurial-scm.org/wiki/MergeStateRecords> for '
>                         'more information'))
>
>     +class UnknownVersion(Abort):
>     +    """generic exception for aborting from an encounter with an
>     unknown version
>     +    """
>     +
>     +    def __init__(self, *args, **kwargs):
>     +        self.version = kwargs.pop('version')
>
>
> Since the arguments are always (msg, version), please define them as
> such and don't abuse **kwargs.

I wanted to preserve the 'Abort' feel of the exception with an option 
"version" argument, but I see you point.

I'll shortly send a V2 with

   def __init__(self, msg, hint=None, version=None):

(note: 'hint' use the same construct[1], but is probably more justified 
in this case)

[1] 
https://www.mercurial-scm.org/repo/hg/file/14e8fef9158d/mercurial/error.py#l24


> Also consider having the constructor
> format an error message from arguments and pass that result to
> Abort.__init__.

I made the exception a bit wider than the current usecase 
(UnknownVersion, not UnknownObsMarkersVersion) as I assume we'll be able 
to reuse it for other usecase. Building the error message in the 
exception itself and keeping it reusable is a bit challenging. I'm 
skipping this in my V2.

Cheers,

-- 
Pierre-Yves David