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

[APPSRE-11112] add verify_ondemend_tests for gitlab-housekeeping #4816

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

BumbleFeng
Copy link
Contributor

@BumbleFeng BumbleFeng commented Jan 20, 2025

allow to check if MR has passed all necessary test jobs and add comments to indicate test results.

@BumbleFeng BumbleFeng force-pushed the ondemand branch 6 times, most recently from 48862e2 to c157a1b Compare January 20, 2025 19:33
@BumbleFeng BumbleFeng changed the title [APPSRE-11112] add verify_ondemend_tests for gitlab-housekeeping WIP[APPSRE-11112] add verify_ondemend_tests for gitlab-housekeeping Jan 20, 2025
@BumbleFeng BumbleFeng force-pushed the ondemand branch 7 times, most recently from c2bb888 to 88c9c8a Compare January 22, 2025 03:59
@BumbleFeng BumbleFeng changed the title WIP[APPSRE-11112] add verify_ondemend_tests for gitlab-housekeeping [APPSRE-11112] add verify_ondemend_tests for gitlab-housekeeping Jan 22, 2025
Copy link
Contributor

@esron esron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines 198 to 202
gl_fork = GitLabApi(
instance=gl_instance,
project_id=mr.source_project_id,
settings=gl_settings,
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason to create a new GitLabApi instead of reuse current gl, GitLabApi instance require cleanup.

Copy link
Contributor Author

@BumbleFeng BumbleFeng Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original gl targets the upstream repo while this gl_fork targets the fork repo. We can only get the commit status from the fork repo. I have switch it using with.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need this part

gitlab_request.labels(integration=INTEGRATION_NAME).inc()
self.project = self.gl.projects.get(project_id)
, right?

maybe worth extract another method get_project_by_id, and replace prevous project_url part with current get_project. This can save calls used to init GitLabApi, and save those paramter passing of gl_instance and gl_settings.

Also need to make request counter correct as commits.get(commit.id) has 1 request, and .statuses.list() has another (if no pagination), need a better way to count mentioned in https://issues.redhat.com/browse/APPSRE-10450

reconcile/gitlab_housekeeping.py Outdated Show resolved Hide resolved
@BumbleFeng BumbleFeng force-pushed the ondemand branch 2 times, most recently from ba6c29c to a164a7f Compare January 23, 2025 15:10
Signed-off-by: Feng Huang <fehuang@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants