Re: [PATCHv1 1/3] git-p4: raise exceptions from p4CmdList based on error from p4 server
>>
>>> ErrorId MsgDb::MaxResults = { ErrorOf( ES_DB, 32,
>>> E_FAILED, EV_ADMIN, 1 ), "Request too large (over %maxResults%); see
>>> 'p4 help maxresults'." } ;//NOTRANS
>>> ErrorId MsgDb::MaxScanRows = { ErrorOf( ES_DB, 61,
>>> E_FAILED, EV_ADMIN, 1 ), "Too many rows scanned (over %maxScanRows%);
>>> see 'p4 help maxscanrows'." } ;//NOTRANS
>>>
>>> I don't think there's actually a way to make it return any language
>>> other than English though. [...]
>>> So I think probably the language is always English.
>>
>> The "NOTRANS" annotation on the error messages is reassuring.
>
> I'll check it works OK on Windows; charset translation might cause a problem.
Works OK on Windows with 2017.2 client/server.
Re: [PATCHv1 1/3] git-p4: raise exceptions from p4CmdList based on error from p4 server
On 5 June 2018 at 20:41, Eric Sunshine wrote:
> On Tue, Jun 5, 2018 at 6:56 AM Luke Diamand wrote:
>> On 5 June 2018 at 10:54, Eric Sunshine wrote:
>> > On Tue, Jun 5, 2018 at 5:14 AM Luke Diamand wrote:
>> >> +m = re.search('Too many rows scanned \(over
>> >> (\d+)\)', data)
>> >> +if not m:
>> >> +m = re.search('Request too large \(over
>> >> (\d+)\)', data)
>> >
>> > Does 'p4' localize these error messages?
>>
>> That's a good question.
>>
>> It turns out that Perforce open-sourced the P4 client in 2014 (I only
>> recently found this out) so we can actually look at the code now!
>>
>> Here's the code:
>>
>> // ErrorId graveyard: retired/deprecated ErrorIds.
>
> Hmm, the "too many rows" error you're seeing is retired/deprecated(?).
There's some code elsewhere that suggests it's being kept alive:
// Retired ErrorIds. We need to keep these so that clients
// built with newer apis can commnunicate with older servers
// still sending these.
static ErrorId MaxResults; // DEPRECATED
static ErrorId MaxScanRows; // DEPRECATED
>
>> ErrorId MsgDb::MaxResults = { ErrorOf( ES_DB, 32,
>> E_FAILED, EV_ADMIN, 1 ), "Request too large (over %maxResults%); see
>> 'p4 help maxresults'." } ;//NOTRANS
>> ErrorId MsgDb::MaxScanRows = { ErrorOf( ES_DB, 61,
>> E_FAILED, EV_ADMIN, 1 ), "Too many rows scanned (over %maxScanRows%);
>> see 'p4 help maxscanrows'." } ;//NOTRANS
>>
>> I don't think there's actually a way to make it return any language
>> other than English though. [...]
>> So I think probably the language is always English.
>
> The "NOTRANS" annotation on the error messages is reassuring.
I'll check it works OK on Windows; charset translation might cause a problem.
Re: [PATCHv1 1/3] git-p4: raise exceptions from p4CmdList based on error from p4 server
On Tue, Jun 5, 2018 at 6:56 AM Luke Diamand wrote:
> On 5 June 2018 at 10:54, Eric Sunshine wrote:
> > On Tue, Jun 5, 2018 at 5:14 AM Luke Diamand wrote:
> >> +m = re.search('Too many rows scanned \(over (\d+)\)',
> >> data)
> >> +if not m:
> >> +m = re.search('Request too large \(over (\d+)\)',
> >> data)
> >
> > Does 'p4' localize these error messages?
>
> That's a good question.
>
> It turns out that Perforce open-sourced the P4 client in 2014 (I only
> recently found this out) so we can actually look at the code now!
>
> Here's the code:
>
> // ErrorId graveyard: retired/deprecated ErrorIds.
Hmm, the "too many rows" error you're seeing is retired/deprecated(?).
> ErrorId MsgDb::MaxResults = { ErrorOf( ES_DB, 32,
> E_FAILED, EV_ADMIN, 1 ), "Request too large (over %maxResults%); see
> 'p4 help maxresults'." } ;//NOTRANS
> ErrorId MsgDb::MaxScanRows = { ErrorOf( ES_DB, 61,
> E_FAILED, EV_ADMIN, 1 ), "Too many rows scanned (over %maxScanRows%);
> see 'p4 help maxscanrows'." } ;//NOTRANS
>
> I don't think there's actually a way to make it return any language
> other than English though. [...]
> So I think probably the language is always English.
The "NOTRANS" annotation on the error messages is reassuring.
Re: [PATCHv1 1/3] git-p4: raise exceptions from p4CmdList based on error from p4 server
On 5 June 2018 at 10:54, Eric Sunshine wrote:
> On Tue, Jun 5, 2018 at 5:14 AM Luke Diamand wrote:
>> This change lays some groundwork for better handling of rowcount errors
>> from the server, where it fails to send us results because we requested
>> too many.
>>
>> It adds an option to p4CmdList() to return errors as a Python exception.
>>
>> The exceptions are derived from P4Exception (something went wrong),
>> P4ServerException (the server sent us an error code) and
>> P4RequestSizeException (we requested too many rows/results from the
>> server database).
>>
>> This makes makes the code that handles the errors a bit easier.
>>
>> The default behavior is unchanged; the new code is enabled with a flag.
>>
>> Signed-off-by: Luke Diamand
>> ---
>> diff --git a/git-p4.py b/git-p4.py
>> @@ -566,10 +566,30 @@ def isModeExec(mode):
>> +class P4ServerException(Exception):
>> +""" Base class for exceptions where we get some kind of marshalled up
>> result from the server """
>> +def __init__(self, exit_code, p4_result):
>> +super(P4ServerException, self).__init__(exit_code)
>> +self.p4_result = p4_result
>> +self.code = p4_result[0]['code']
>> +self.data = p4_result[0]['data']
>
> The subsequent patches never seem to access any of these fields, so
> it's difficult to judge whether it's worthwhile having 'code' and
> 'data' bits split out like this.
These changes don't use it, but I thought that future changes might
need them, and perhaps when I put that code in I was thinking I might
need it myself.
>
>> @@ -616,9 +636,25 @@ def p4CmdList(cmd, stdin=None, stdin_mode='w+b',
>> cb=None, skip_info=False):
>> if exitCode != 0:
>> -entry = {}
>> -entry["p4ExitCode"] = exitCode
>> -result.append(entry)
>> +if errors_as_exceptions:
>> +if len(result) > 0:
>> +data = result[0].get('data')
>> +if data:
>> +m = re.search('Too many rows scanned \(over (\d+)\)',
>> data)
>> +if not m:
>> +m = re.search('Request too large \(over (\d+)\)',
>> data)
>
> Does 'p4' localize these error messages?
That's a good question.
The marshalled-up error from Perforce looks like this:
([{'generic': 35, 'code': 'error', 'data': "Too many rows scanned
(over 40); see 'p4 help maxscanrows'.\n", 'severity': 3}])
It turns out that Perforce open-sourced the P4 client in 2014 (I only
recently found this out) so we can actually look at the code now!
https://swarm.workshop.perforce.com/projects/perforce_software-p4
Clone it like this:
mkdir p4 &&
(cd p4 && git init && git config --add git-p4.branchList p4/2018-1:2018-1) &&
P4USER=guest P4PORT=workshop.perforce.com:1666 git p4 clone
--detect-branches --destination p4 //guest/perforce_software/p4@all
Here's the code:
// ErrorId graveyard: retired/deprecated ErrorIds.
ErrorId MsgDb::MaxResults = { ErrorOf( ES_DB, 32,
E_FAILED, EV_ADMIN, 1 ), "Request too large (over %maxResults%); see
'p4 help maxresults'." } ;//NOTRANS
ErrorId MsgDb::MaxScanRows = { ErrorOf( ES_DB, 61,
E_FAILED, EV_ADMIN, 1 ), "Too many rows scanned (over %maxScanRows%);
see 'p4 help maxscanrows'." } ;//NOTRANS
I don't think there's actually a way to make it return any language
other than English though. There's a P4LANGUAGE environment variable,
but it just says "this is for system integrators":
https://www.perforce.com/perforce/r15.2/manuals/cmdref/P4LANGUAGE.html
So I think probably the language is always English.
Luke
Re: [PATCHv1 1/3] git-p4: raise exceptions from p4CmdList based on error from p4 server
On Tue, Jun 5, 2018 at 5:14 AM Luke Diamand wrote:
> This change lays some groundwork for better handling of rowcount errors
> from the server, where it fails to send us results because we requested
> too many.
>
> It adds an option to p4CmdList() to return errors as a Python exception.
>
> The exceptions are derived from P4Exception (something went wrong),
> P4ServerException (the server sent us an error code) and
> P4RequestSizeException (we requested too many rows/results from the
> server database).
>
> This makes makes the code that handles the errors a bit easier.
>
> The default behavior is unchanged; the new code is enabled with a flag.
>
> Signed-off-by: Luke Diamand
> ---
> diff --git a/git-p4.py b/git-p4.py
> @@ -566,10 +566,30 @@ def isModeExec(mode):
> +class P4ServerException(Exception):
> +""" Base class for exceptions where we get some kind of marshalled up
> result from the server """
> +def __init__(self, exit_code, p4_result):
> +super(P4ServerException, self).__init__(exit_code)
> +self.p4_result = p4_result
> +self.code = p4_result[0]['code']
> +self.data = p4_result[0]['data']
The subsequent patches never seem to access any of these fields, so
it's difficult to judge whether it's worthwhile having 'code' and
'data' bits split out like this.
> @@ -616,9 +636,25 @@ def p4CmdList(cmd, stdin=None, stdin_mode='w+b',
> cb=None, skip_info=False):
> if exitCode != 0:
> -entry = {}
> -entry["p4ExitCode"] = exitCode
> -result.append(entry)
> +if errors_as_exceptions:
> +if len(result) > 0:
> +data = result[0].get('data')
> +if data:
> +m = re.search('Too many rows scanned \(over (\d+)\)',
> data)
> +if not m:
> +m = re.search('Request too large \(over (\d+)\)',
> data)
Does 'p4' localize these error messages?
> +if m:
> +limit = int(m.group(1))
> +raise P4RequestSizeException(exitCode, result, limit)
> +
> +raise P4ServerException(exitCode, result)
> +else:
> +raise P4Exception(exitCode)
> +else:
> +entry = {}
> +entry["p4ExitCode"] = exitCode
> +result.append(entry)
