PR#1402: frontend: don't raise 500 on misconfigured build-time repos - copr/copr

copr / copr

#1402 frontend: don't raise 500 on misconfigured build-time repos

Merged 3 years ago by praiskup. Opened 3 years ago by praiskup.

Unknown source error-500-build-config into master

frontend: don't raise 500 on misconfigured build-time repos

Pavel Raiskup • 3 years ago

c9b0eeb

frontend/coprs_frontend/coprs/logic/complex_logic.py

file modified

+10 -2

		`@@ -394,7 +394,6 @@`
		`"name": "Copr buildroot",`
		`})`

		`-`
		`repos.extend(cls.get_additional_repo_views(copr.repos_list, chroot_id))`
		`repos.extend(cls.get_additional_repo_views(chroot.repos_list, chroot_id))`

		`@@ -419,7 +418,16 @@`
		`"name": "Additional repo " + helpers.generate_repo_name(repo),`
		`}`

		`- copr = ComplexLogic.get_copr_by_repo_safe(repo)`
		`+ # We ask get_copr_by_repo_safe() here only to resolve the`
		`+ # module_hotfixes attribute. If the asked project doesn't exist, we`
		`+ # still adjust the 'repos' variable -- the build will eventually`
		`+ # fail on repo downloading, but at least the copr maintainer will be`
		`+ # notified about the misconfiguration. Better than just skip the`
		`+ # repo.`
		`+ try:`
		`+ copr = ComplexLogic.get_copr_by_repo_safe(repo)`
frostyx commented 3 years ago Meh, the `ComplexLogic` is so inconsistent. This method is not supposed to raise exceptions (but it does) but rather return `None`.
frostyx commented 3 years ago I created #1408 so we eventually might clean it up. I think it might be a good easy fix issue for a potential high school intern. Or we can decide to drop the `_safe` convention entirely if there is a better way to do it.
		`+ except ObjectNotFound:`
		`+ copr = None`
		`if copr and copr.module_hotfixes:`
		`params["module_hotfixes"] = True`

frontend/coprs_frontend/tests/test_logic/test_complex_logic.py

file modified

+27 -1

		`@@ -7,7 +7,11 @@`
		`from coprs import models, helpers, app`
		`from copr_common.enums import ActionTypeEnum`
		`from coprs.logic.actions_logic import ActionsLogic`
		`- from coprs.logic.complex_logic import ComplexLogic, ProjectForking`
		`+ from coprs.logic.complex_logic import (`
		`+ BuildConfigLogic,`
		`+ ComplexLogic,`
		`+ ProjectForking,`
		`+ )`
		`from coprs.logic.coprs_logic import CoprChrootsLogic`
		`from tests.coprs_test_case import CoprsTestCase, new_app_context`

		`@@ -173,6 +177,28 @@`
		`assert ComplexLogic.get_copr_by_repo_safe("copr:///user1/foocopr") == None`
		`assert ComplexLogic.get_copr_by_repo_safe("copr://user1//foocopr") == None`

		`+ @new_app_context`
		`+ @pytest.mark.usefixtures("f_users", "f_coprs", "f_mock_chroots", "f_builds",`
		`+ "f_db")`
		`+ def test_generate_build_config_with_dep_mistake(self):`
		`+ bcl = BuildConfigLogic`
		`+ main_repo = {`
		`+ "id": "copr_base",`
		`+ "name": "Copr repository",`
		`+ "baseurl": "http://copr-be-dev.cloud.fedoraproject.org"`
		`+ "/results/user1/foocopr/fedora-18-x86_64/",`
		`+ }`
		`+ build_config = bcl.generate_build_config(self.c1, "fedora-18-x86_64")`
		`+ assert build_config["repos"] == [main_repo]`
		`+`
		`+ self.c1.repos = "copr://non/existing"`
		`+ build_config = bcl.generate_build_config(self.c1, "fedora-18-x86_64")`
		`+`
		`+ # We put the link there, even though baseurl points to 404. The build`
		`+ # will later fail on downloading the repository and user will be`
		`+ # notified.`
		`+ assert len(build_config["repos"]) == 2`
		`+ assert build_config["repos"][1]["id"] == "copr_non_existing"`

		`class FooModel(object):`
		`"""`