-
-
Notifications
You must be signed in to change notification settings - Fork 937
Pr cmd raise with stderr on error #452
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5077fc7
78f3f38
46201b3
08a0fad
3565888
4a5ceba
6891caf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -173,13 +173,17 @@ class RemoteProgress(object): | |
DONE_TOKEN = 'done.' | ||
TOKEN_SEPARATOR = ', ' | ||
|
||
__slots__ = ("_cur_line", "_seen_ops") | ||
__slots__ = ("_cur_line", "_seen_ops", "_error_lines") | ||
re_op_absolute = re.compile(r"(remote: )?([\w\s]+):\s+()(\d+)()(.*)") | ||
re_op_relative = re.compile(r"(remote: )?([\w\s]+):\s+(\d+)% \((\d+)/(\d+)\)(.*)") | ||
|
||
def __init__(self): | ||
self._seen_ops = list() | ||
self._cur_line = None | ||
self._error_lines = [] | ||
|
||
def get_stderr(self): | ||
return '\n'.join(self._error_lines) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I recommend providing the highest-value data available, which in this case in an array of lines. There should be no assumption about how the user will evaluate that data. |
||
|
||
def _parse_progress_line(self, line): | ||
"""Parse progress information from the given line as retrieved by git-push | ||
|
@@ -190,6 +194,10 @@ def _parse_progress_line(self, line): | |
# Counting objects: 4, done. | ||
# Compressing objects: 50% (1/2) \rCompressing objects: 100% (2/2) \rCompressing objects: 100% (2/2), done. | ||
self._cur_line = line | ||
if len(self._error_lines) > 0 or self._cur_line.startswith(('error:', 'fatal:')): | ||
self._error_lines.append(self._cur_line) | ||
return [] | ||
|
||
sub_lines = line.split('\r') | ||
failed_lines = list() | ||
for sline in sub_lines: | ||
|
@@ -764,10 +772,10 @@ def done(self): | |
self.cv.notify_all() | ||
self.cv.release() | ||
|
||
def wait(self): | ||
def wait(self, stderr=b''): | ||
self.cv.acquire() | ||
while self.count > 0: | ||
self.cv.wait() | ||
self.cv.wait(strerr=stderr) | ||
self.cv.release() | ||
|
||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure it' s enough to break after the first line indicating a problem ? My intuition is that the lines that follow on stderr contain context, that you probably want to catch as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works because of the 2nd break at line 591.
Also I see this work in testing.