Fix fedora-review error handling #2835
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
frontend/coprs_frontend/alembic/versions/bf4b5dc74740_map_mock_croots_to_dits_git_branch.py
Fixed
Show fixed
Hide fixed
FrostyX
force-pushed
the
fix-fedora-review
branch
4 times, most recently
from
3547398
to
eaa27f8
Compare
praiskup
reviewed
Aug 7, 2023
| @@ -10,6 +10,7 @@ | |||
|
|
|||
| import os | |||
| import shutil | |||
| from copr_common.helpers import chroot_to_branch | |||
There was a problem hiding this comment.
This change is no longer needed because we won't use chroot_to_branch in copr-rpmbuild after all. But since you +1'd it, I left the commit there.
praiskup
reviewed
Aug 7, 2023
| cmd = [ | ||
| "fedora-review", "--no-colors", "--prebuilt", "--rpm-spec", | ||
| "--name", self.package_name, | ||
| "--mock-config", self.mock_config_file, | ||
| "--define", "DISTTAG={0}".format(branch), |
There was a problem hiding this comment.
This must be some bug in fedora-review, what am I missing? Is the package actually having the dist tag or not? (this must be bothering normal fedora-review users, not just Copr)
|
I made this way harder than it needed to be.
|
FrostyX
force-pushed
the
fix-fedora-review
branch
from
eaa27f8
to
9c6e396
Compare
praiskup
pushed a commit
to praiskup/copr
that referenced
this pull request
Aug 8, 2023
Fix FrostyX/fedora-review-service#28 Fix https://pagure.io/FedoraReview/issue/486 There is no fedora-review parameter for specifying the output directory. As a work-around we are renaming its results directory to "fedora-review" after the command finishes. However, our error handling is wrong and if the `fedora-review` command fails, the rename doesn't happend. Therefore, broken links. This commit fixes it. Relates: fedora-copr#2835
|
/packit build |
FrostyX
force-pushed
the
fix-fedora-review
branch
from
9c6e396
to
d76cb08
Compare
FrostyX
changed the title
Fix FrostyX/fedora-review-service#28 Fix https://pagure.io/FedoraReview/issue/486 There is no fedora-review parameter for specifying the output directory. As a work-around we are renaming its results directory to "fedora-review" after the command finishes. However, our error handling is wrong and if the `fedora-review` command fails, the rename doesn't happend. Therefore, broken links. This commit fixes it.
FrostyX
force-pushed
the
fix-fedora-review
branch
from
d76cb08
to
cc16d8b
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix FrostyX/fedora-review-service#28
Fix https://pagure.io/FedoraReview/issue/486
The
fedora-reviewcommands fails with the following error:It is strange to me why it cannot find any disttag in the package but
it is easy for us to specify it, so why not.