On 5 June 2018 at 10:54, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > On Tue, Jun 5, 2018 at 5:14 AM Luke Diamand <luke@xxxxxxxxxxx> 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 <luke@xxxxxxxxxxx> >> --- >> 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