Skip to content
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

Needs improved error handling and validation for automation #153

Closed
amimas opened this issue Dec 21, 2020 · 5 comments
Closed

Needs improved error handling and validation for automation #153

amimas opened this issue Dec 21, 2020 · 5 comments
Labels
Milestone

Comments

@amimas
Copy link
Collaborator

amimas commented Dec 21, 2020

I'm relatively new to GitlabForm. While setting up GitlabForm I noticed that the error handling is not consistent.

For example: if I provide an invalid config file path, GitlabForm quits with exit code 1. But, if my configuration is invalid, GitlabForm quits with exit code 0.

Right now my config mentions a project that does not exist. As a result, when GitlabForm is executed, it shows NotFoundException error but does not quit with a non-zero exit code.

+++ Error while processing 'my-group/my-project'
Traceback (most recent call last):
  File "/gitlabform/gitlabform/gitlabform/core.py", line 315, in process_all
    self.project_processors.process_project(
  File "/gitlabform/gitlabform/gitlabform/processors/project/__init__.py", line 57, in process_project
    processor.process(project_and_group, configuration, dry_run)
  File "/gitlabform/gitlabform/gitlabform/processors/util/decorators.py", line 42, in method_wrapper
    return method(self, project_and_group, SafeDict(configuration), dry_run)
  File "/gitlabform/gitlabform/gitlabform/processors/abstract_processor.py", line 34, in process
    self._process_configuration(project_or_project_and_group, configuration)
  File "/gitlabform/gitlabform/gitlabform/processors/project/merge_requests_processor.py", line 18, in _process_configuration
    self.gitlab.post_approvals_settings(project_and_group, approvals)
  File "/gitlabform/gitlabform/gitlab/projects.py", line 205, in post_approvals_settings
    pid = self._get_project_id(project_and_group_name)
  File "/gitlabform/gitlabform/gitlab/core.py", line 103, in _get_project_id
    result = self.get_project(project_and_group)
  File "/gitlabform/gitlabform/gitlab/core.py", line 78, in get_project
    return self._make_requests_to_api("projects/%s", project_and_group_or_id)
  File "/gitlabform/gitlabform/gitlab/core.py", line 137, in _make_requests_to_api
    response = self._make_request_to_api(
  File "/gitlabform/gitlabform/gitlab/core.py", line 220, in _make_request_to_api
    raise NotFoundException("Resource path='%s' not found!" % url)
gitlabform.gitlab.core.NotFoundException: Resource path='***/api/v4/projects/my-group%2Fmy-project' not found!
/project # 
/project # 
/project # echo $?
0

I had another error in the configuration which had also resulted in following exception but GitlabForm did not quit with a non-zero exit code.

raise UnexpectedResponseException(
gitlabform.gitlab.core.UnexpectedResponseException: Request url='https://***/api/v4/projects/my-group%2Fproject-example', method=PUT, data='None' failed - expected code(s) [200], got code 400 & body: 'b'{"error":"public_builds is invalid, packages_enabled is invalid"}''

This will be considered as successful if we rely on applying this configuration through GitLab CI pipeline (after change are merged or a scheduled pipeline). That means we can't rely on the pipeline's status. I have to manually inspect the log for every execution.

In addition, if I run the same config file (containing invalid project name) with --noop parameter for a dry run, it runs successfully without any error, which is misleading in this case. That means I can't rely on the dry run for an MR build or a branch build before config changes are merged.

This issue could probably be split into 2. I'm curious to know if others have come across these issues and what workaround is being used, if any.

@gdubicki
Copy link
Member

gdubicki commented Dec 26, 2020

Thanks for reporting this @amimas !

We really do want this app to use proper exit codes as we run it in CI too.

I'll check it out and first try to fix all the cases with exit code 0 while it should be non-zero.

Then I’ll take a look at the dry run behavior.

@gdubicki gdubicki changed the title Needs improvd error handling and validation for automation Needs improved error handling and validation for automation Dec 26, 2020
@gdubicki
Copy link
Member

Sorry for keeping this on hold for so long, @amimas .

Originally Gitlabform was ignoring all errors.
This came from our own experience from using it. When we were running it for, lets say, 100 repositories, then we didn't want to have repos 30-70 skipped just because Gitlabform failed on repo no 29.

But some users wanted a different behaviour, so we added it in #137 - now with -t / --terminate you can request Gitlabform to stop on the first error (and return a proper non-zero exit code).

Now I think that we need another feature, mutually exclusive with the above one: proceed forward on errors, but at the end exit with non-zero. I will implement it along with possible cleanup of the exit codes based on this issue. :)

gdubicki pushed a commit that referenced this issue Mar 26, 2021
(This is a breaking change because GitLabForm v1.x exited with 0
in such case!)

List the groups and projects that have failed, with their numbers.
Add --start-from-group to allow retries from a given group number.
(We used to have only --start-from for projects.)
Always print the stacktraces on processing errors.
Unify and document (in -h output) the exit codes used.

Fixes #153.
@gdubicki
Copy link
Member

On second thought the "proceed forward on errors, but at the end exit with non-zero" should be the default behavior as this is the right thing to do. If someone wants to ignore those errors, then they can always do gitlabform (...) | true.

As this is technically a backward-incompatible change I used this as an excuse to kick-off work on v2.0. 😅

2.0.0b1 contains these changes: main...v2.0.0b1

Can you please try out this version, @amimas ?

gdubicki pushed a commit that referenced this issue May 1, 2021
(This is a breaking change because GitLabForm v1.x exited with 0
in such case!)

List the groups and projects that have failed, with their numbers.
Add --start-from-group to allow retries from a given group number.
(We used to have only --start-from for projects.)
Always print the stacktraces on processing errors.
Unify and document (in -h output) the exit codes used.

Fixes #153.
@gdubicki gdubicki added this to the v2.0 milestone May 15, 2021
@gdubicki
Copy link
Member

v2 final has been released. Please reopen this ticket if you still have issues.

@amimas
Copy link
Collaborator Author

amimas commented May 23, 2021

Thanks @gdubicki !! I haven't had a chance to try out v2 yet. I agree with the exit strategy mentioned above and glad to hear it's been implemented. Can't wait to try it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants