Skip to content

chore: make reset_gitlab() better #1785

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 22, 2022
Merged

chore: make reset_gitlab() better #1785

merged 1 commit into from
Jul 22, 2022

Conversation

JohnVillalovos
Copy link
Member

@JohnVillalovos JohnVillalovos commented Dec 29, 2021 •
edited
Loading

Saw issues in the CI where reset_gitlab() would fail. It would fail to
delete the group that is created when GitLab starts up. Extending the
timeout didn't fix the issue.

Changed the code to use the new helpers.safe_delete() function.
Which will delete the resource and then make sure it is deleted before
returning.

Also added some logging functionality that can be seen if logging is
turned on in pytest.

@JohnVillalovos JohnVillalovos marked this pull request as draft December 29, 2021 05:42
@JohnVillalovos JohnVillalovos changed the title wip: make reset_gitlab() better chore: make reset_gitlab() better Dec 29, 2021
@JohnVillalovos JohnVillalovos marked this pull request as ready for review December 29, 2021 08:06
@JohnVillalovos JohnVillalovos requested a review from nejch December 29, 2021 08:07
@JohnVillalovos
Copy link
Member Author

JohnVillalovos commented Dec 29, 2021 •
edited
Loading

@nejch Please look at the functional testing logs and let me know what you think.

@JohnVillalovos
Copy link
Member Author

JohnVillalovos commented Dec 29, 2021 •
edited
Loading

An example of what this is fixing from https://github.com/python-gitlab/python-gitlab/runs/4656774636?check_suite_focus=true#step:5:27

tests/functional/api/test_clusters.py::test_project_clusters 
-------------------------------- live log setup --------------------------------
2021-12-29 08:13:06.617 [    INFO] (conftest.py:gl:L258) Create python-gitlab gitlab.Gitlab object
2021-12-29 08:13:07.660 [    INFO] (conftest.py:_reset_gitlab_delete_resources:L35) Mark for deletion project: 'Monitoring'
2021-12-29 08:13:08.277 [    INFO] (conftest.py:_reset_gitlab_delete_resources:L44) Mark for deletion group: 'GitLab Instance'
2021-12-29 08:13:08.684 [    INFO] (conftest.py:wait_for_maximum_list_length:L83) Iteration: 0: items in projects: []
2021-12-29 08:13:08.954 [    INFO] (conftest.py:wait_for_maximum_list_length:L83) Iteration: 0: items in groups: ['GitLab Instance']

# NOTE(jlvillal): Deleting again and no message about already marked for deletion, so this is a fix.
2021-12-29 08:13:08.955 [    INFO] (conftest.py:wait_for_maximum_list_length:L88) Marking again for deletion groups: 'GitLab Instance'
2021-12-29 08:13:09.142 [    INFO] (conftest.py:wait_for_maximum_list_length:L83) Iteration: 1: items in groups: ['GitLab Instance']

# NOTE(jlvillal): 2nd time through, deleting again but this time indicates already marked for deletion.
2021-12-29 08:13:09.143 [    INFO] (conftest.py:wait_for_maximum_list_length:L88) Marking again for deletion groups: 'GitLab Instance'
2021-12-29 08:13:09.177 [    INFO] (conftest.py:wait_for_maximum_list_length:L94) Already marked for deletion: 'GitLab Instance' 404: 404 Group Not Found

# NOTE(jlvillal): 3rd time through and group is now gone.
2021-12-29 08:13:09.308 [    INFO] (conftest.py:wait_for_maximum_list_length:L83) Iteration: 2: items in groups: []
2021-12-29 08:13:09.359 [    INFO] (conftest.py:wait_for_maximum_list_length:L83) Iteration: 0: items in variables: []
2021-12-29 08:13:09.525 [    INFO] (conftest.py:wait_for_maximum_list_length:L83) Iteration: 0: items in users: ['Administrator']

@JohnVillalovos JohnVillalovos force-pushed the jlvillal/reset_gitlab branch 2 times, most recently from 3c5f510 to cda90f0 Compare December 29, 2021 17:14
@JohnVillalovos JohnVillalovos marked this pull request as draft December 30, 2021 05:50
@JohnVillalovos JohnVillalovos force-pushed the jlvillal/reset_gitlab branch 3 times, most recently from 26f4944 to 67dd7b4 Compare December 31, 2021 21:14
@JohnVillalovos JohnVillalovos force-pushed the jlvillal/reset_gitlab branch from 67dd7b4 to 517a330 Compare June 2, 2022 04:39
@JohnVillalovos JohnVillalovos marked this pull request as ready for review June 2, 2022 05:30
@JohnVillalovos JohnVillalovos force-pushed the jlvillal/reset_gitlab branch from 517a330 to 48710b7 Compare June 12, 2022 16:29
@JohnVillalovos JohnVillalovos force-pushed the jlvillal/reset_gitlab branch from 48710b7 to 0480644 Compare June 20, 2022 17:40
@JohnVillalovos JohnVillalovos force-pushed the jlvillal/reset_gitlab branch from 0480644 to 6bc9dfd Compare June 30, 2022 00:57
@JohnVillalovos JohnVillalovos mentioned this pull request Jul 1, 2022
Merged
@JohnVillalovos JohnVillalovos force-pushed the jlvillal/reset_gitlab branch 2 times, most recently from e9e8a7c to 9b4c01b Compare July 22, 2022 15:21
@JohnVillalovos
chore: make reset_gitlab() better
d87d6b1 
Saw issues in the CI where reset_gitlab() would fail. It would fail to
delete the group that is created when GitLab starts up. Extending the
timeout didn't fix the issue.

Changed the code to use the new `helpers.safe_delete()` function.
Which will delete the resource and then make sure it is deleted before
returning.

Also added some logging functionality that can be seen if logging is
turned on in pytest.
@JohnVillalovos JohnVillalovos force-pushed the jlvillal/reset_gitlab branch from 9b4c01b to d87d6b1 Compare July 22, 2022 15:23
@JohnVillalovos
Copy link
Member Author

@nejch By using safe_delete() it simplified things 🙂

@nejch
Copy link
Member

nejch commented Jul 22, 2022

@nejch By using safe_delete() it simplified things slightly_smiling_face

Amazing, love to see code get deleted 😅

nejch
nejch reviewed Jul 22, 2022
View reviewed changes
@nejch nejch merged commit 789ef81 into main Jul 22, 2022
@nejch nejch deleted the jlvillal/reset_gitlab branch July 22, 2022 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nejch nejch nejch left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@JohnVillalovos @nejch