{"thread":{"id":"48654","subject":"[PATCHv1 0/3] git-p4: improvements to p4 \"blocking\"","startedAt":"2018-06-05T09:14:01Z","lastAt":"2018-06-08T10:07:00Z","messageCount":10,"participants":["Luke Diamand","Eric Sunshine"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"349325","messageId":"20180605091350.14476-1-luke@diamand.org","threadId":"48654","inReplyTo":null,"subject":"[PATCHv1 0/3] git-p4: improvements to p4 \"blocking\"","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2018-06-05T09:13:47Z","receivedAt":"2018-06-05T09:14:01Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"This patchset improves the way that git-p4 sends requests in \"blocks\".\n\nThe problem comes about because the Perforce server limits the maximum\nnumber of results it will send back (there are actually two different\nlimits).\n\nThis causes git-p4 failures if you ask for too much data.\n\nIn commit 96b2d54aee (\"git-p4: use -m when running p4 changes\",\n2015-04-20), git-p4 learned to make requests in blocks, by default 512\nin size.\n\nThe problem with this, however, is that it can be very slow on large\nrepositories - you might have millions of commits, although only a\nhandful actually relate to the code you are trying to clone. Each 512\nblock request takes a sizeable fraction of a second to complete.\n\nThere's a command-line option to increase the block size, but unless you\nknow about it, it won't be at all obvious that you could use this to\nspeed things up.\n\nThis change\n~~~~~~~~~~~\n\nThe function p4CmdList() has been taught to raise an exception when it\ngets an error, and in particular, to notice and report the two \"too many\nresults\" errors.\n\nThe code that does the blocking can now start out with a nice large\nblock size, and then reduce it if it sees an error.\n\nLuke Diamand (3):\n git-p4: raise exceptions from p4CmdList based on error from p4 server\n git-p4: narrow the scope of exceptions caught when parsing an int\n git-p4: auto-size the block\n\n git-p4.py | 72 +++++++++++++++++++++++++++++++++++------\n t/t9818-git-p4-block.sh | 11 +++++++\n 2 files changed, 73 insertions(+), 10 deletions(-)\n\n-- \n2.17.0.392.gdeb1a6e9b7\n\n"},{"id":"349326","messageId":"20180605091350.14476-2-luke@diamand.org","threadId":"48654","inReplyTo":"20180605091350.14476-1-luke@diamand.org","subject":"[PATCHv1 1/3] git-p4: raise exceptions from p4CmdList based on error from p4 server","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2018-06-05T09:13:48Z","receivedAt":"2018-06-05T09:14:03Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"This change lays some groundwork for better handling of rowcount errors\nfrom the server, where it fails to send us results because we requested\ntoo many.\n\nIt adds an option to p4CmdList() to return errors as a Python exception.\n\nThe exceptions are derived from P4Exception (something went wrong),\nP4ServerException (the server sent us an error code) and\nP4RequestSizeException (we requested too many rows/results from the\nserver database).\n\nThis makes makes the code that handles the errors a bit easier.\n\nThe default behavior is unchanged; the new code is enabled with a flag.\n\nSigned-off-by: Luke Diamand \n---\n git-p4.py | 44 ++++++++++++++++++++++++++++++++++++++++----\n 1 file changed, 40 insertions(+), 4 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 66652f8280..d856078ec8 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -566,10 +566,30 @@ def isModeExec(mode):\n # otherwise False.\n return mode[-3:] == \"755\"\n \n+class P4Exception(Exception):\n+ \"\"\" Base class for exceptions from the p4 client \"\"\"\n+ def __init__(self, exit_code):\n+ self.p4ExitCode = exit_code\n+\n+class P4ServerException(Exception):\n+ \"\"\" Base class for exceptions where we get some kind of marshalled up result from the server \"\"\"\n+ def __init__(self, exit_code, p4_result):\n+ super(P4ServerException, self).__init__(exit_code)\n+ self.p4_result = p4_result\n+ self.code = p4_result[0]['code']\n+ self.data = p4_result[0]['data']\n+\n+class P4RequestSizeException(P4ServerException):\n+ \"\"\" One of the maxresults or maxscanrows errors \"\"\"\n+ def __init__(self, exit_code, p4_result, limit):\n+ super(P4RequestSizeException, self).__init__(exit_code, p4_result)\n+ self.limit = limit\n+\n def isModeExecChanged(src_mode, dst_mode):\n return isModeExec(src_mode) != isModeExec(dst_mode)\n \n-def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False):\n+def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False,\n+ errors_as_exceptions=False):\n \n if isinstance(cmd,basestring):\n cmd = \"-G \" + cmd\n@@ -616,9 +636,25 @@ def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False):\n pass\n exitCode = p4.wait()\n if exitCode != 0:\n- entry = {}\n- entry[\"p4ExitCode\"] = exitCode\n- result.append(entry)\n+ if errors_as_exceptions:\n+ if len(result) > 0:\n+ data = result[0].get('data')\n+ if data:\n+ m = re.search('Too many rows scanned \\(over (\\d+)\\)', data)\n+ if not m:\n+ m = re.search('Request too large \\(over (\\d+)\\)', data)\n+\n+ if m:\n+ limit = int(m.group(1))\n+ raise P4RequestSizeException(exitCode, result, limit)\n+\n+ raise P4ServerException(exitCode, result)\n+ else:\n+ raise P4Exception(exitCode)\n+ else:\n+ entry = {}\n+ entry[\"p4ExitCode\"] = exitCode\n+ result.append(entry)\n \n return result\n \n-- \n2.17.0.392.gdeb1a6e9b7\n\n"},{"id":"349327","messageId":"20180605091350.14476-3-luke@diamand.org","threadId":"48654","inReplyTo":"20180605091350.14476-1-luke@diamand.org","subject":"[PATCHv1 2/3] git-p4: narrow the scope of exceptions caught when parsing an int","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2018-06-05T09:13:49Z","receivedAt":"2018-06-05T09:14:05Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"The current code traps all exceptions around some code which parses an\ninteger, and then talks to Perforce.\n\nThat can result in errors from Perforce being ignored. Change the code\nto only catch the integer conversion exceptions.\n\nSigned-off-by: Luke Diamand \n---\n git-p4.py | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex d856078ec8..b337562b39 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -959,7 +959,7 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):\n try:\n (changeStart, changeEnd) = p4ParseNumericChangeRange(parts)\n block_size = chooseBlockSize(requestedBlockSize)\n- except:\n+ except ValueError:\n changeStart = parts[0][1:]\n changeEnd = parts[1]\n if requestedBlockSize:\n-- \n2.17.0.392.gdeb1a6e9b7\n\n"},{"id":"349328","messageId":"20180605091350.14476-4-luke@diamand.org","threadId":"48654","inReplyTo":"20180605091350.14476-1-luke@diamand.org","subject":"[PATCHv1 3/3] git-p4: auto-size the block","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2018-06-05T09:13:50Z","receivedAt":"2018-06-05T09:14:09Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"git-p4 originally would fetch changes in one query. On large repos this\ncould fail because of the limits that Perforce imposes on the number of\nitems returned and the number of queries in the database.\n\nTo fix this, git-p4 learned to query changes in blocks of 512 changes,\nHowever, this can be very slow - if you have a few million changes,\nwith each chunk taking about a second, it can be an hour or so.\n\nAlthough it's possible to tune this value manually with the\n\"--changes-block-size\" option, it's far from obvious to ordinary users\nthat this is what needs doing.\n\nThis change alters the block size dynamically by looking for the\nspecific error messages returned from the Perforce server, and reducing\nthe block size if the error is seen, either to the limit reported by the\nserver, or to half the current block size.\n\nThat means we can start out with a very large block size, and then let\nit automatically drop down to a value that works without error, while\nstill failing correctly if some other error occurs.\n\nSigned-off-by: Luke Diamand \n---\n git-p4.py | 26 +++++++++++++++++++++-----\n t/t9818-git-p4-block.sh | 11 +++++++++++\n 2 files changed, 32 insertions(+), 5 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex b337562b39..6736f81b60 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -48,7 +48,8 @@ def __str__(self):\n defaultLabelRegexp = r'[a-zA-Z0-9_\\-.]+$'\n \n # Grab changes in blocks of this many revisions, unless otherwise requested\n-defaultBlockSize = 512\n+# The code should now automatically reduce the block size if it is required\n+defaultBlockSize = 1<<20\n \n p4_access_checked = False\n \n@@ -969,7 +970,8 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):\n changes = set()\n \n # Retrieve changes a block at a time, to prevent running\n- # into a MaxResults/MaxScanRows error from the server.\n+ # into a MaxResults/MaxScanRows error from the server. If\n+ # we _do_ hit one of those errors, turn down the block size\n \n while True:\n cmd = ['changes']\n@@ -983,10 +985,24 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):\n for p in depotPaths:\n cmd += [\"%s...@%s\" % (p, revisionRange)]\n \n+ # fetch the changes\n+ try:\n+ result = p4CmdList(cmd, errors_as_exceptions=True)\n+ except P4RequestSizeException as e:\n+ if not block_size:\n+ block_size = e.limit\n+ elif block_size > e.limit:\n+ block_size = e.limit\n+ else:\n+ block_size = max(2, block_size // 2)\n+\n+ if verbose: print(\"block size error, retry with block size {0}\".format(block_size))\n+ continue\n+ except P4Exception as e:\n+ die('Error retrieving changes description ({0})'.format(e.p4ExitCode))\n+\n # Insert changes in chronological order\n- for entry in reversed(p4CmdList(cmd)):\n- if entry.has_key('p4ExitCode'):\n- die('Error retrieving changes descriptions ({})'.format(entry['p4ExitCode']))\n+ for entry in reversed(result):\n if not entry.has_key('change'):\n continue\n changes.add(int(entry['change']))\ndiff --git a/t/t9818-git-p4-block.sh b/t/t9818-git-p4-block.sh\nindex 8840a183ac..e5ec9cdec8 100755\n--- a/t/t9818-git-p4-block.sh\n+++ b/t/t9818-git-p4-block.sh\n@@ -129,6 +129,7 @@ test_expect_success 'Create a repo with multiple depot paths' '\n '\n \n test_expect_success 'Clone repo with multiple depot paths' '\n+\ttest_when_finished cleanup_git &&\n \t(\n \t\tcd \"$git\" &&\n \t\tgit p4 clone --changes-block-size=4 //depot/pathA@all //depot/pathB@all \\\n@@ -138,6 +139,16 @@ test_expect_success 'Clone repo with multiple depot paths' '\n \t)\n '\n \n+test_expect_success 'Clone repo with self-sizing block size' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone --changes-block-size=1000000 //depot@all --destination=\"$git\" &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit log --oneline > log &&\n+\t\ttest_line_count \\> 10 log\n+\t)\n+'\n+\n test_expect_success 'kill p4d' '\n \tkill_p4d\n '\n-- \n2.17.0.392.gdeb1a6e9b7\n\n"},{"id":"349329","messageId":"CAPig+cQTTY8wc4d=4jV8GxHDOjF7xk1vjA62JS6s4Zr0uXiegg@mail.gmail.com","threadId":"48654","inReplyTo":"20180605091350.14476-2-luke@diamand.org","subject":"Re: [PATCHv1 1/3] git-p4: raise exceptions from p4CmdList based on error from p4 server","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-06-05T09:54:01Z","receivedAt":"2018-06-05T09:54:17Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Jun 5, 2018 at 5:14 AM Luke Diamand wrote:\n> This change lays some groundwork for better handling of rowcount errors\n> from the server, where it fails to send us results because we requested\n> too many.\n>\n> It adds an option to p4CmdList() to return errors as a Python exception.\n>\n> The exceptions are derived from P4Exception (something went wrong),\n> P4ServerException (the server sent us an error code) and\n> P4RequestSizeException (we requested too many rows/results from the\n> server database).\n>\n> This makes makes the code that handles the errors a bit easier.\n>\n> The default behavior is unchanged; the new code is enabled with a flag.\n>\n> Signed-off-by: Luke Diamand \n> ---\n> diff --git a/git-p4.py b/git-p4.py\n> @@ -566,10 +566,30 @@ def isModeExec(mode):\n> +class P4ServerException(Exception):\n> + \"\"\" Base class for exceptions where we get some kind of marshalled up result from the server \"\"\"\n> + def __init__(self, exit_code, p4_result):\n> + super(P4ServerException, self).__init__(exit_code)\n> + self.p4_result = p4_result\n> + self.code = p4_result[0]['code']\n> + self.data = p4_result[0]['data']\n\nThe subsequent patches never seem to access any of these fields, so\nit's difficult to judge whether it's worthwhile having 'code' and\n'data' bits split out like this.\n\n> @@ -616,9 +636,25 @@ def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False):\n> if exitCode != 0:\n> - entry = {}\n> - entry[\"p4ExitCode\"] = exitCode\n> - result.append(entry)\n> + if errors_as_exceptions:\n> + if len(result) > 0:\n> + data = result[0].get('data')\n> + if data:\n> + m = re.search('Too many rows scanned \\(over (\\d+)\\)', data)\n> + if not m:\n> + m = re.search('Request too large \\(over (\\d+)\\)', data)\n\nDoes 'p4' localize these error messages?\n\n> + if m:\n> + limit = int(m.group(1))\n> + raise P4RequestSizeException(exitCode, result, limit)\n> +\n> + raise P4ServerException(exitCode, result)\n> + else:\n> + raise P4Exception(exitCode)\n> + else:\n> + entry = {}\n> + entry[\"p4ExitCode\"] = exitCode\n> + result.append(entry)\n"},{"id":"349330","messageId":"CAPig+cQj1jxgHKMagkX71v3W7KHbkZOrEJ_aceY_LsiSWVt_yw@mail.gmail.com","threadId":"48654","inReplyTo":"20180605091350.14476-4-luke@diamand.org","subject":"Re: [PATCHv1 3/3] git-p4: auto-size the block","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-06-05T10:05:00Z","receivedAt":"2018-06-05T10:05:14Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Jun 5, 2018 at 5:14 AM Luke Diamand wrote:\n> git-p4 originally would fetch changes in one query. On large repos this\n> could fail because of the limits that Perforce imposes on the number of\n> items returned and the number of queries in the database.\n>\n> To fix this, git-p4 learned to query changes in blocks of 512 changes,\n> However, this can be very slow - if you have a few million changes,\n> with each chunk taking about a second, it can be an hour or so.\n>\n> Although it's possible to tune this value manually with the\n> \"--changes-block-size\" option, it's far from obvious to ordinary users\n> that this is what needs doing.\n>\n> This change alters the block size dynamically by looking for the\n> specific error messages returned from the Perforce server, and reducing\n> the block size if the error is seen, either to the limit reported by the\n> server, or to half the current block size.\n>\n> That means we can start out with a very large block size, and then let\n> it automatically drop down to a value that works without error, while\n> still failing correctly if some other error occurs.\n>\n> Signed-off-by: Luke Diamand \n> ---\n> diff --git a/git-p4.py b/git-p4.py\n> @@ -48,7 +48,8 @@ def __str__(self):\n> # Grab changes in blocks of this many revisions, unless otherwise requested\n> -defaultBlockSize = 512\n> +# The code should now automatically reduce the block size if it is required\n> +defaultBlockSize = 1<<20\n\nAs worded, the new comment only has value to someone who knew the old\nbehavior (before this patch), not for someone reading the code fresh.\nHowever, if reworded, it might be meaningful to all readers (new and\nold):\n\n # The block size is reduced automatically if required\n\n> @@ -983,10 +985,24 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):\n> + try:\n> + result = p4CmdList(cmd, errors_as_exceptions=True)\n> + except P4RequestSizeException as e:\n> + if not block_size:\n> + block_size = e.limit\n> + elif block_size > e.limit:\n> + block_size = e.limit\n> + else:\n> + block_size = max(2, block_size // 2)\n> +\n> + if verbose: print(\"block size error, retry with block size {0}\".format(block_size))\n\nPerhaps: s/retry/retrying/\n\n> diff --git a/t/t9818-git-p4-block.sh b/t/t9818-git-p4-block.sh\n> @@ -129,6 +129,7 @@ test_expect_success 'Create a repo with multiple depot paths' '\n> +test_expect_success 'Clone repo with self-sizing block size' '\n> + test_when_finished cleanup_git &&\n> + git p4 clone --changes-block-size=1000000 //depot@all --destination=\"$git\" &&\n> + (\n> + cd \"$git\" &&\n> + git log --oneline > log &&\n> + test_line_count \\> 10 log\n> + )\n> +'\n\nOr, without a subshell (and dropping whitespace after redirection operator):\n\n git -C git log --oneline >log &&\n test_line_count \\> 10 log\n\n(not worth a re-roll)\n"},{"id":"349331","messageId":"CAE5ih7-1StB8RNrobO+hpG2QOSdoscUNfVP9+muZV0_b+m+XSw@mail.gmail.com","threadId":"48654","inReplyTo":"CAPig+cQTTY8wc4d=4jV8GxHDOjF7xk1vjA62JS6s4Zr0uXiegg@mail.gmail.com","subject":"Re: [PATCHv1 1/3] git-p4: raise exceptions from p4CmdList based on error from p4 server","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2018-06-05T10:56:14Z","receivedAt":"2018-06-05T10:56:17Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 5 June 2018 at 10:54, Eric Sunshine wrote:\n> On Tue, Jun 5, 2018 at 5:14 AM Luke Diamand wrote:\n>> This change lays some groundwork for better handling of rowcount errors\n>> from the server, where it fails to send us results because we requested\n>> too many.\n>>\n>> It adds an option to p4CmdList() to return errors as a Python exception.\n>>\n>> The exceptions are derived from P4Exception (something went wrong),\n>> P4ServerException (the server sent us an error code) and\n>> P4RequestSizeException (we requested too many rows/results from the\n>> server database).\n>>\n>> This makes makes the code that handles the errors a bit easier.\n>>\n>> The default behavior is unchanged; the new code is enabled with a flag.\n>>\n>> Signed-off-by: Luke Diamand \n>> ---\n>> diff --git a/git-p4.py b/git-p4.py\n>> @@ -566,10 +566,30 @@ def isModeExec(mode):\n>> +class P4ServerException(Exception):\n>> + \"\"\" Base class for exceptions where we get some kind of marshalled up result from the server \"\"\"\n>> + def __init__(self, exit_code, p4_result):\n>> + super(P4ServerException, self).__init__(exit_code)\n>> + self.p4_result = p4_result\n>> + self.code = p4_result[0]['code']\n>> + self.data = p4_result[0]['data']\n>\n> The subsequent patches never seem to access any of these fields, so\n> it's difficult to judge whether it's worthwhile having 'code' and\n> 'data' bits split out like this.\n\nThese changes don't use it, but I thought that future changes might\nneed them, and perhaps when I put that code in I was thinking I might\nneed it myself.\n\n>\n>> @@ -616,9 +636,25 @@ def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False):\n>> if exitCode != 0:\n>> - entry = {}\n>> - entry[\"p4ExitCode\"] = exitCode\n>> - result.append(entry)\n>> + if errors_as_exceptions:\n>> + if len(result) > 0:\n>> + data = result[0].get('data')\n>> + if data:\n>> + m = re.search('Too many rows scanned \\(over (\\d+)\\)', data)\n>> + if not m:\n>> + m = re.search('Request too large \\(over (\\d+)\\)', data)\n>\n> Does 'p4' localize these error messages?\n\nThat's a good question.\n\nThe marshalled-up error from Perforce looks like this:\n\n ([{'generic': 35, 'code': 'error', 'data': \"Too many rows scanned\n(over 40); see 'p4 help maxscanrows'.\\n\", 'severity': 3}])\n\nIt turns out that Perforce open-sourced the P4 client in 2014 (I only\nrecently found this out) so we can actually look at the code now!\n\n https://swarm.workshop.perforce.com/projects/perforce_software-p4\n\nClone it like this:\n\nmkdir p4 &&\n(cd p4 && git init && git config --add git-p4.branchList p4/2018-1:2018-1) &&\nP4USER=guest P4PORT=workshop.perforce.com:1666 git p4 clone\n--detect-branches --destination p4 //guest/perforce_software/p4@all\n\nHere's the code:\n\n // ErrorId graveyard: retired/deprecated ErrorIds.\n\n ErrorId MsgDb::MaxResults = { ErrorOf( ES_DB, 32,\nE_FAILED, EV_ADMIN, 1 ), \"Request too large (over %maxResults%); see\n'p4 help maxresults'.\" } ;//NOTRANS\n ErrorId MsgDb::MaxScanRows = { ErrorOf( ES_DB, 61,\nE_FAILED, EV_ADMIN, 1 ), \"Too many rows scanned (over %maxScanRows%);\nsee 'p4 help maxscanrows'.\" } ;//NOTRANS\n\n\nI don't think there's actually a way to make it return any language\nother than English though. There's a P4LANGUAGE environment variable,\nbut it just says \"this is for system integrators\":\n\nhttps://www.perforce.com/perforce/r15.2/manuals/cmdref/P4LANGUAGE.html\n\nSo I think probably the language is always English.\n\nLuke\n"},{"id":"349380","messageId":"CAPig+cTSrzmOtYPwPWLpGfDa5tt0SV6Aq8SmhYTTicK0ikst7Q@mail.gmail.com","threadId":"48654","inReplyTo":"CAE5ih7-1StB8RNrobO+hpG2QOSdoscUNfVP9+muZV0_b+m+XSw@mail.gmail.com","subject":"Re: [PATCHv1 1/3] git-p4: raise exceptions from p4CmdList based on error from p4 server","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-06-05T19:41:12Z","receivedAt":"2018-06-05T19:41:29Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Jun 5, 2018 at 6:56 AM Luke Diamand wrote:\n> On 5 June 2018 at 10:54, Eric Sunshine wrote:\n> > On Tue, Jun 5, 2018 at 5:14 AM Luke Diamand wrote:\n> >> + m = re.search('Too many rows scanned \\(over (\\d+)\\)', data)\n> >> + if not m:\n> >> + m = re.search('Request too large \\(over (\\d+)\\)', data)\n> >\n> > Does 'p4' localize these error messages?\n>\n> That's a good question.\n>\n> It turns out that Perforce open-sourced the P4 client in 2014 (I only\n> recently found this out) so we can actually look at the code now!\n>\n> Here's the code:\n>\n> // ErrorId graveyard: retired/deprecated ErrorIds.\n\nHmm, the \"too many rows\" error you're seeing is retired/deprecated(?).\n\n> ErrorId MsgDb::MaxResults = { ErrorOf( ES_DB, 32,\n> E_FAILED, EV_ADMIN, 1 ), \"Request too large (over %maxResults%); see\n> 'p4 help maxresults'.\" } ;//NOTRANS\n> ErrorId MsgDb::MaxScanRows = { ErrorOf( ES_DB, 61,\n> E_FAILED, EV_ADMIN, 1 ), \"Too many rows scanned (over %maxScanRows%);\n> see 'p4 help maxscanrows'.\" } ;//NOTRANS\n>\n> I don't think there's actually a way to make it return any language\n> other than English though. [...]\n> So I think probably the language is always English.\n\nThe \"NOTRANS\" annotation on the error messages is reassuring.\n"},{"id":"349416","messageId":"CAE5ih7-HMRJ3nFNuQCXmJEC4Ar4PZY=_4Q=4gDK+VTFRhMvCGA@mail.gmail.com","threadId":"48654","inReplyTo":"CAPig+cTSrzmOtYPwPWLpGfDa5tt0SV6Aq8SmhYTTicK0ikst7Q@mail.gmail.com","subject":"Re: [PATCHv1 1/3] git-p4: raise exceptions from p4CmdList based on error from p4 server","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2018-06-05T22:14:29Z","receivedAt":"2018-06-05T22:14:33Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 5 June 2018 at 20:41, Eric Sunshine wrote:\n> On Tue, Jun 5, 2018 at 6:56 AM Luke Diamand wrote:\n>> On 5 June 2018 at 10:54, Eric Sunshine wrote:\n>> > On Tue, Jun 5, 2018 at 5:14 AM Luke Diamand wrote:\n>> >> + m = re.search('Too many rows scanned \\(over (\\d+)\\)', data)\n>> >> + if not m:\n>> >> + m = re.search('Request too large \\(over (\\d+)\\)', data)\n>> >\n>> > Does 'p4' localize these error messages?\n>>\n>> That's a good question.\n>>\n>> It turns out that Perforce open-sourced the P4 client in 2014 (I only\n>> recently found this out) so we can actually look at the code now!\n>>\n>> Here's the code:\n>>\n>> // ErrorId graveyard: retired/deprecated ErrorIds.\n>\n> Hmm, the \"too many rows\" error you're seeing is retired/deprecated(?).\n\nThere's some code elsewhere that suggests it's being kept alive:\n\n // Retired ErrorIds. We need to keep these so that clients\n // built with newer apis can commnunicate with older servers\n // still sending these.\n\n static ErrorId MaxResults; // DEPRECATED\n static ErrorId MaxScanRows; // DEPRECATED\n\n\n>\n>> ErrorId MsgDb::MaxResults = { ErrorOf( ES_DB, 32,\n>> E_FAILED, EV_ADMIN, 1 ), \"Request too large (over %maxResults%); see\n>> 'p4 help maxresults'.\" } ;//NOTRANS\n>> ErrorId MsgDb::MaxScanRows = { ErrorOf( ES_DB, 61,\n>> E_FAILED, EV_ADMIN, 1 ), \"Too many rows scanned (over %maxScanRows%);\n>> see 'p4 help maxscanrows'.\" } ;//NOTRANS\n>>\n>> I don't think there's actually a way to make it return any language\n>> other than English though. [...]\n>> So I think probably the language is always English.\n>\n> The \"NOTRANS\" annotation on the error messages is reassuring.\n\nI'll check it works OK on Windows; charset translation might cause a problem.\n"},{"id":"349687","messageId":"CAE5ih7-4CTOgaWuc2dVha5bcF1MsD+ERPj3hQNTOddOrkvMZWw@mail.gmail.com","threadId":"48654","inReplyTo":"CAE5ih7-HMRJ3nFNuQCXmJEC4Ar4PZY=_4Q=4gDK+VTFRhMvCGA@mail.gmail.com","subject":"Re: [PATCHv1 1/3] git-p4: raise exceptions from p4CmdList based on error from p4 server","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2018-06-08T10:06:56Z","receivedAt":"2018-06-08T10:07:00Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":">>\n>>> ErrorId MsgDb::MaxResults = { ErrorOf( ES_DB, 32,\n>>> E_FAILED, EV_ADMIN, 1 ), \"Request too large (over %maxResults%); see\n>>> 'p4 help maxresults'.\" } ;//NOTRANS\n>>> ErrorId MsgDb::MaxScanRows = { ErrorOf( ES_DB, 61,\n>>> E_FAILED, EV_ADMIN, 1 ), \"Too many rows scanned (over %maxScanRows%);\n>>> see 'p4 help maxscanrows'.\" } ;//NOTRANS\n>>>\n>>> I don't think there's actually a way to make it return any language\n>>> other than English though. [...]\n>>> So I think probably the language is always English.\n>>\n>> The \"NOTRANS\" annotation on the error messages is reassuring.\n>\n> I'll check it works OK on Windows; charset translation might cause a problem.\n\nWorks OK on Windows with 2017.2 client/server.\n"}]}