Do not run jobs when manual_trigger=True #2069

Frawless · 2023-05-26T11:44:19Z

This PR adds logic that when user specify manual_trigger: True in job definition, the handler will not consider this job as executable automatically, only via comment.

TODO:

Write new tests or update the old ones to cover new functionality.
Update or write new documentation in packit/packit.dev.

Fixes #1806

Related to packit/packit#1984

Merge after packit/packit#1984

RELEASE NOTES BEGIN
TODO
RELEASE NOTES END

softwarefactory-project-zuul · 2023-05-26T12:01:53Z

Build failed.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/dd544d259f0f4ccba90c28c92b5b8bdf

✔️ pre-commit SUCCESS in 2m 11s
❌ packit-service-tests FAILURE in 3m 06s
❌ packit-service-tests-openshift FAILURE in 17m 12s

lbarcziova

thanks again for the contribution!

packit_service/worker/jobs.py

softwarefactory-project-zuul · 2023-05-26T19:22:03Z

Build failed.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/f5cf612e6bdd4253ae746c956ece94a3

❌ pre-commit FAILURE in 2m 13s
❌ packit-service-tests FAILURE in 3m 01s
❌ packit-service-tests-openshift FAILURE in 17m 21s

lbarcziova

regarding the tests, I would add few test cases here... if you need help with that, we can do that :)

we also run pre-commit in the CI that enforces rebased branch, see

lbarcziova · 2023-05-29T07:38:11Z

packit_service/worker/jobs.py

@@ -615,7 +631,8 @@ def get_jobs_matching_event(self) -> List[JobConfig]:
                )
                and job not in jobs_matching_trigger
            ):
-                jobs_matching_trigger.append(job)
+                if not job.manual_trigger or self.event in MANUAL_EVENTS:


as I am looking into the list of MANUAL_EVENTS, we could reduce the events to only AbstractCommentEvent, and CheckRerunEvent there and use any(isinstance(self.event, event_type) for event_type in MANUAL_EVENTS) here to count with inheritance

lbarcziova · 2023-05-29T07:38:51Z

packit_service/worker/handlers/testing_farm.py

@@ -77,6 +78,7 @@
 @reacts_to(PushGitlabEvent)
 @reacts_to(MergeRequestGitlabEvent)
 @reacts_to(AbstractPRCommentEvent)
+@reacts_to(AbstractIssueCommentEvent)


we want to react only to pull request comments for running the Testing Farm

I understood that some of the users want to trigger it from the issues. Is that something we will deal with in another pr? Or I just misunderstood it?

we allow retriggering from issues for the propose_downstream job

softwarefactory-project-zuul · 2023-05-29T10:44:53Z

Build failed.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/a920e7170c8944eca41da830b3a28033

❌ pre-commit FAILURE in 2m 08s
❌ packit-service-tests FAILURE in 2m 17s
❌ packit-service-tests-openshift FAILURE in 16m 50s

mfocko

LGTM :) Thanks a lot for your contribution ❤️

mfocko · 2023-05-29T13:30:52Z

packit_service/worker/jobs.py

+                    or any(
+                        isinstance(self.event, event_type)
+                        for event_type in MANUAL_EVENTS
+                    )
+                    or job.trigger != JobConfigTriggerType.pull_request


Suggested change

or any(

isinstance(self.event, event_type)

for event_type in MANUAL_EVENTS

)

or job.trigger != JobConfigTriggerType.pull_request

or job.trigger != JobConfigTriggerType.pull_request

or any(

isinstance(self.event, event_type)

for event_type in MANUAL_EVENTS

)

Just my OCD, I guess, but .trigger seems to be something that can be resolved faster than the any(…)

is the job.trigger != JobConfigTriggerType.pull_request acutally needed?

in case user will specify manual_trigger=True in jobs that do not care about the option we should skip it...now I am thinking if checking job type is a better option or not 🤔 something like job.type != JobType.tests...or is this all handled by decorators here

packit-service/packit_service/worker/handlers/testing_farm.py

Line 70 in 38d6158

@run_for_comment(command="test")

?

yes, the decorators should take care of the mapping itself

my thinking was that by omitting that part of the condition this could be used generally for any job (e.g. also for the mentioned propose-downstream) without binding it to the testing farm job (+ the docs would explain the option properly); @mfocko WDYT?

+1 for having it for all commands; if it gets used in a job that is not easily run via comment, it's a user error IMO, not an issue 🤷

Also I looked around, not only at the diff, I would probably merge it with the if in which it's wrapped now; keeping the last check of the if above at the end even after merging it in one condition. (It just seems like an unnecessary indentation)

Got it, I updated the code and tests.

mfocko · 2023-05-29T13:31:23Z

packit_service/worker/jobs.py


        jobs_matching_trigger.extend(self.check_explicit_matching())

+        print(jobs_matching_trigger)


Suggested change

print(jobs_matching_trigger)

logger.debug(jobs_matching_trigger)

In case, it's only for your testing, please don't forget to remove it :)

Ye just leftover from debugging, removed!

softwarefactory-project-zuul · 2023-05-29T13:32:46Z

Build failed.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/e9300fd0168f4f69886c71af7a6f441d

✔️ pre-commit SUCCESS in 2m 11s
❌ packit-service-tests FAILURE in 2m 10s
❌ packit-service-tests-openshift RETRY_LIMIT in 1m 45s

softwarefactory-project-zuul · 2023-05-29T13:51:49Z

Build failed.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/fb7fce65ed1647059905250249994bc8

✔️ pre-commit SUCCESS in 2m 11s
❌ packit-service-tests FAILURE in 2m 12s
❌ packit-service-tests-openshift FAILURE in 15m 59s

softwarefactory-project-zuul · 2023-05-29T16:47:08Z

Build failed.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/85b30ad1fa604daaad3516e61c238788

✔️ pre-commit SUCCESS in 2m 11s
❌ packit-service-tests FAILURE in 2m 18s
❌ packit-service-tests-openshift FAILURE in 16m 16s

lbarcziova

LGTM once the tests are green 🚀

Add new option manual_trigger to packit schema This PR adds new option manual_trigger into Packit schema to allow users to trigger Packit jobs manually. Service integration done in packit/packit-service#2069 Write new tests or update the old ones to cover new functionality. Update or write new documentation in packit/packit.dev. Fixes packit/packit-service#1806 RELEASE NOTES BEGIN Packit now supports automatic ordering of ☕ after all checks pass. RELEASE NOTES END Reviewed-by: Laura Barcziová

lbarcziova · 2023-05-29T18:47:10Z

recheck

softwarefactory-project-zuul · 2023-05-29T19:00:10Z

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/3b5784b5bc0a40a4ac1720227295f575

✔️ pre-commit SUCCESS in 2m 07s
✔️ packit-service-tests SUCCESS in 2m 36s
✔️ packit-service-tests-openshift SUCCESS in 12m 37s

lbarcziova

nice, thanks!!

could you please squash the commits here as well?

Frawless · 2023-05-30T06:38:35Z

@lbarcziova done, thanks

Signed-off-by: Jakub Stejskal <xstejs24@gmail.com>

softwarefactory-project-zuul · 2023-05-30T06:49:54Z

Build failed.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/ca94b674c6de4ce2b5e9ca4e44e4c1cf

❌ pre-commit FAILURE in 2m 11s
✔️ packit-service-tests SUCCESS in 2m 29s
✔️ packit-service-tests-openshift SUCCESS in 12m 34s

softwarefactory-project-zuul · 2023-05-30T07:05:31Z

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/7fffb9963948468d877063ddefe1aac4

✔️ pre-commit SUCCESS in 2m 10s
✔️ packit-service-tests SUCCESS in 2m 26s
✔️ packit-service-tests-openshift SUCCESS in 12m 56s

softwarefactory-project-zuul · 2023-05-30T07:15:54Z

Build succeeded (gate pipeline).
https://softwarefactory-project.io/zuul/t/packit-service/buildset/b61cb63504c644e98f1ac7dbd32e56ea

✔️ pre-commit SUCCESS in 9m 21s

Fix results events reporting from packit-service Fixes issue introduced in #2069. When manual_trigger=True the results event are filtered and the results are not propagated properly back to je github/gitlab. Write new tests or update the old ones to cover new functionality. Reviewed-by: Laura Barcziová Reviewed-by: Jakub Stejskal <xstejs24@gmail.com> Reviewed-by: František Lachman

lbarcziova reviewed May 26, 2023

View reviewed changes

packit_service/worker/jobs.py Outdated Show resolved Hide resolved

lbarcziova reviewed May 29, 2023

View reviewed changes

Frawless force-pushed the packit-manual-trigger branch 2 times, most recently from 5281e12 to 82299a3 Compare May 29, 2023 10:24

Frawless force-pushed the packit-manual-trigger branch 2 times, most recently from 363d581 to 6a1315d Compare May 29, 2023 13:25

mfocko reviewed May 29, 2023

View reviewed changes

Frawless mentioned this pull request May 29, 2023

Add new option manual_trigger to packit schema packit/packit#1984

Merged

2 tasks

lbarcziova reviewed May 29, 2023

View reviewed changes

Frawless marked this pull request as ready for review May 29, 2023 19:01

lbarcziova approved these changes May 29, 2023

View reviewed changes

Frawless force-pushed the packit-manual-trigger branch from e4935b1 to e6247a2 Compare May 30, 2023 06:37

Do not run jobs when manual_trigger=True

6bb2c6c

Signed-off-by: Jakub Stejskal <xstejs24@gmail.com>

lbarcziova force-pushed the packit-manual-trigger branch from e6247a2 to 6bb2c6c Compare May 30, 2023 06:52

lbarcziova added the mergeit When set, zuul wil gate and merge the PR. label May 30, 2023

softwarefactory-project-zuul bot merged commit 0556932 into packit:main May 30, 2023

Frawless mentioned this pull request Jun 1, 2023

Fix results events reporting from packit-service #2075

Merged

1 task

Frawless mentioned this pull request Jun 1, 2023

Testing farm and packit integration strimzi/strimzi-kafka-operator#8565

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Do not run jobs when manual_trigger=True #2069

Do not run jobs when manual_trigger=True #2069

Frawless commented May 26, 2023 •

edited

Loading

softwarefactory-project-zuul bot commented May 26, 2023

lbarcziova left a comment

softwarefactory-project-zuul bot commented May 26, 2023

lbarcziova left a comment

lbarcziova May 29, 2023

lbarcziova May 29, 2023

Frawless May 29, 2023

lbarcziova May 29, 2023

softwarefactory-project-zuul bot commented May 29, 2023

mfocko left a comment

mfocko May 29, 2023

Frawless May 29, 2023

lbarcziova May 29, 2023

Frawless May 29, 2023

lbarcziova May 29, 2023

mfocko May 29, 2023

Frawless May 29, 2023

mfocko May 29, 2023

Frawless May 29, 2023

softwarefactory-project-zuul bot commented May 29, 2023

softwarefactory-project-zuul bot commented May 29, 2023

softwarefactory-project-zuul bot commented May 29, 2023

lbarcziova left a comment

lbarcziova commented May 29, 2023

softwarefactory-project-zuul bot commented May 29, 2023

lbarcziova left a comment

Frawless commented May 30, 2023

softwarefactory-project-zuul bot commented May 30, 2023

softwarefactory-project-zuul bot commented May 30, 2023

softwarefactory-project-zuul bot commented May 30, 2023


		jobs_matching_trigger.extend(self.check_explicit_matching())

		print(jobs_matching_trigger)

	print(jobs_matching_trigger)
	logger.debug(jobs_matching_trigger)

Do not run jobs when manual_trigger=True #2069

Do not run jobs when manual_trigger=True #2069

Conversation

Frawless commented May 26, 2023 • edited Loading

softwarefactory-project-zuul bot commented May 26, 2023

lbarcziova left a comment

Choose a reason for hiding this comment

softwarefactory-project-zuul bot commented May 26, 2023

lbarcziova left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

softwarefactory-project-zuul bot commented May 29, 2023

mfocko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

softwarefactory-project-zuul bot commented May 29, 2023

softwarefactory-project-zuul bot commented May 29, 2023

softwarefactory-project-zuul bot commented May 29, 2023

lbarcziova left a comment

Choose a reason for hiding this comment

lbarcziova commented May 29, 2023

softwarefactory-project-zuul bot commented May 29, 2023

lbarcziova left a comment

Choose a reason for hiding this comment

Frawless commented May 30, 2023

softwarefactory-project-zuul bot commented May 30, 2023

softwarefactory-project-zuul bot commented May 30, 2023

softwarefactory-project-zuul bot commented May 30, 2023

Frawless commented May 26, 2023 •

edited

Loading