[PATCH 0/3] gitlab-pipeline-status script: provide more information on errors

Cleber Rosa posted 3 patches 3 years, 2 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210222193240.921250-1-crosa@redhat.com
Maintainers: Fam Zheng <fam@euphon.net>, "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <philmd@redhat.com>, Wainer dos Santos Moschetta <wainersm@redhat.com>, Thomas Huth <thuth@redhat.com>
scripts/ci/gitlab-pipeline-status | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
[PATCH 0/3] gitlab-pipeline-status script: provide more information on errors
Posted by Cleber Rosa 3 years, 2 months ago
When things go wrong with the GitLab API requests, it's useful to give
users more information about the possible causes.

Cleber Rosa (3):
  scripts/ci/gitlab-pipeline-status: split utlity function for HTTP GET
  scripts/ci/gitlab-pipeline-status: give more information on failures
  scripts/ci/gitlab-pipeline-status: give more info when pipeline not
    found

 scripts/ci/gitlab-pipeline-status | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

-- 
2.25.4



Re: [PATCH 0/3] gitlab-pipeline-status script: provide more information on errors
Posted by Wainer dos Santos Moschetta 3 years, 2 months ago
Hi Cleber,

In case you need to send a v2, mind to add the following patch together?

commit 3c4ed8a78e096e4d7df0398c29887a9d468ae120 (HEAD -> gitlab_runners)
Author: Wainer dos Santos Moschetta <wainersm@redhat.com>
Date:   Tue Feb 23 11:26:08 2021 -0300

     scripts/ci/gitlab-pipeline-status: Handle ValueError exceptions nicely

     With this change, when getting the local branch, it will handle nicely
     any threw ValueError exception instead of print the stack trace.

     Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

diff --git a/scripts/ci/gitlab-pipeline-status 
b/scripts/ci/gitlab-pipeline-status
index 924db327ff..6177df973a 100755
--- a/scripts/ci/gitlab-pipeline-status
+++ b/scripts/ci/gitlab-pipeline-status
@@ -160,7 +160,11 @@ def main():
      args = parser.parse_args()

      if not args.commit:
-        args.commit = get_local_branch_commit(args.branch)
+        try:
+            args.commit = get_local_branch_commit(args.branch)
+        except ValueError as error:
+            print("ERROR: %s" % error)
+            sys.exit(1)

      success = False
      try:

On 2/22/21 4:32 PM, Cleber Rosa wrote:
> When things go wrong with the GitLab API requests, it's useful to give
> users more information about the possible causes.
>
> Cleber Rosa (3):
>    scripts/ci/gitlab-pipeline-status: split utlity function for HTTP GET
>    scripts/ci/gitlab-pipeline-status: give more information on failures
>    scripts/ci/gitlab-pipeline-status: give more info when pipeline not
>      found
>
>   scripts/ci/gitlab-pipeline-status | 25 ++++++++++++++++++-------
>   1 file changed, 18 insertions(+), 7 deletions(-)
>


Re: [PATCH 0/3] gitlab-pipeline-status script: provide more information on errors
Posted by Erik Skultety 3 years, 2 months ago
On Tue, Feb 23, 2021 at 11:52:17AM -0300, Wainer dos Santos Moschetta wrote:
> Hi Cleber,
> 
> In case you need to send a v2, mind to add the following patch together?
> 
> commit 3c4ed8a78e096e4d7df0398c29887a9d468ae120 (HEAD -> gitlab_runners)
> Author: Wainer dos Santos Moschetta <wainersm@redhat.com>
> Date:   Tue Feb 23 11:26:08 2021 -0300
> 
>     scripts/ci/gitlab-pipeline-status: Handle ValueError exceptions nicely
> 
>     With this change, when getting the local branch, it will handle nicely
>     any threw ValueError exception instead of print the stack trace.
> 
>     Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> 
> diff --git a/scripts/ci/gitlab-pipeline-status
> b/scripts/ci/gitlab-pipeline-status
> index 924db327ff..6177df973a 100755
> --- a/scripts/ci/gitlab-pipeline-status
> +++ b/scripts/ci/gitlab-pipeline-status
> @@ -160,7 +160,11 @@ def main():
>      args = parser.parse_args()
> 
>      if not args.commit:
> -        args.commit = get_local_branch_commit(args.branch)
> +        try:
> +            args.commit = get_local_branch_commit(args.branch)
> +        except ValueError as error:
> +            print("ERROR: %s" % error)
> +            sys.exit(1)

1 is the default error code, so you should pass the error message to sys.exit
directly without the print. If you don't want that, at least redirect the
print to sys.stderr.

Erik


Re: [PATCH 0/3] gitlab-pipeline-status script: provide more information on errors
Posted by Wainer dos Santos Moschetta 3 years, 2 months ago
Hi,

On 2/23/21 12:19 PM, Erik Skultety wrote:
> On Tue, Feb 23, 2021 at 11:52:17AM -0300, Wainer dos Santos Moschetta wrote:
>> Hi Cleber,
>>
>> In case you need to send a v2, mind to add the following patch together?
>>
>> commit 3c4ed8a78e096e4d7df0398c29887a9d468ae120 (HEAD -> gitlab_runners)
>> Author: Wainer dos Santos Moschetta <wainersm@redhat.com>
>> Date:   Tue Feb 23 11:26:08 2021 -0300
>>
>>      scripts/ci/gitlab-pipeline-status: Handle ValueError exceptions nicely
>>
>>      With this change, when getting the local branch, it will handle nicely
>>      any threw ValueError exception instead of print the stack trace.
>>
>>      Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
>>
>> diff --git a/scripts/ci/gitlab-pipeline-status
>> b/scripts/ci/gitlab-pipeline-status
>> index 924db327ff..6177df973a 100755
>> --- a/scripts/ci/gitlab-pipeline-status
>> +++ b/scripts/ci/gitlab-pipeline-status
>> @@ -160,7 +160,11 @@ def main():
>>       args = parser.parse_args()
>>
>>       if not args.commit:
>> -        args.commit = get_local_branch_commit(args.branch)
>> +        try:
>> +            args.commit = get_local_branch_commit(args.branch)
>> +        except ValueError as error:
>> +            print("ERROR: %s" % error)
>> +            sys.exit(1)
> 1 is the default error code, so you should pass the error message to sys.exit
> directly without the print. If you don't want that, at least redirect the
> print to sys.stderr.

There are occurrences of the same pattern on the script, so I better 
send a separate series to change them all. Thanks for pointing it out Erik!

- Wainer

>
> Erik


Re: [PATCH 0/3] gitlab-pipeline-status script: provide more information on errors
Posted by Thomas Huth 3 years, 2 months ago
On 22/02/2021 20.32, Cleber Rosa wrote:
> When things go wrong with the GitLab API requests, it's useful to give
> users more information about the possible causes.
> 
> Cleber Rosa (3):
>    scripts/ci/gitlab-pipeline-status: split utlity function for HTTP GET
>    scripts/ci/gitlab-pipeline-status: give more information on failures
>    scripts/ci/gitlab-pipeline-status: give more info when pipeline not
>      found
> 
>   scripts/ci/gitlab-pipeline-status | 25 ++++++++++++++++++-------
>   1 file changed, 18 insertions(+), 7 deletions(-)
> 

Thanks, I've queued the three patches now to my testing-next branch:

https://gitlab.com/thuth/qemu/-/commits/testing-next/

  Thomas