-
Notifications
You must be signed in to change notification settings - Fork 49
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
Migrate to org.fedoraproject.prod.pagure.git.receive #2351
Migrate to org.fedoraproject.prod.pagure.git.receive #2351
Conversation
Signed-off-by: Nikola Forró <[email protected]>
if not self.contains_specfile_change(): | ||
return False | ||
|
||
if self.pull_request: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nforro as you mentioned, there is no way to tell from the message whether the commit(s) is coming from PR, do you think it is ok to always try to get the PR via end_commit
or do you have some other suggestions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we have a choice. But I think this has to be adjusted as well:
packit-service/packit_service/worker/mixin.py
Line 257 in a439cb5
if not self._pull_request and self.data.event_dict["committer"] == "pagure": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we have a choice.
there is still option to revisit the 2 config options, but that would require changes in configs for users who use these options
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering if it still makes sense to differentiate between the two events. But we need to be able to recognize Packit PRs, and the person making the push (agent) is not packit, e.g.: https://apps.fedoraproject.org/datagrepper/v2/id?id=a934e891-c91c-4d7e-b287-3d7ef6771f8f&is_raw=true&size=extra-large.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I know, therefore I wrote it would require changes in the configs (I meant having only one option, for the committer, but the default could not be packit
and would be more complicated). I will go with the current implementation and if we notice some problems with this, we can revisit.
Build succeeded. ✔️ pre-commit SUCCESS in 2m 01s |
c9804c4
to
00c94d1
Compare
Build succeeded. ✔️ pre-commit SUCCESS in 2m 02s |
The check for changed specfile is now again done only in fedmsg as the message now directly contains always the info about changed files. The agent for PR is now not equal to `pagure`, so we cannot rely to check on that and need to try to get the PR via the commit SHA.
00c94d1
to
8a8d1e1
Compare
Build succeeded. ✔️ pre-commit SUCCESS in 2m 04s |
Build succeeded (gate pipeline). ✔️ pre-commit SUCCESS in 2m 03s |
bd38a84
into
packit:main
Supersedes #2303
Related to #2305
RELEASE NOTES BEGIN
N/A
RELEASE NOTES END