PR#3: API for translating module NVR to DNF repo - copr/copr

copr / copr

#3 API for translating module NVR to DNF repo

Merged 7 years ago by clime. Opened 7 years ago by frostyx.

https://github.com/FrostyX/copr.git modularity-nvr-to-repo into master

[frontend] fix module repo tests

Jakub Kadlčík • 7 years ago

46ed3a5

[frontend][python] get module repo for given architecture

Jakub Kadlčík • 7 years ago

9eda785

[frontend] move modules_url as repo_url to Module class

Jakub Kadlčík • 7 years ago

1ec5934

[frontend][python] request project name from client

Jakub Kadlčík • 7 years ago

5497b1f

[frontend] implement method for querying copr by owner

Jakub Kadlčík • 7 years ago

8b67cc6

[frontend] return URL to module data on backend

Jakub Kadlčík • 7 years ago

402d7d9

[python] add method for fetching /api/module/repo

Jakub Kadlčík • 7 years ago

69b1538

[frontend] add api method for translating module NVR to DNF repo url

Jakub Kadlčík • 7 years ago

776fb1e

frontend/coprs_frontend/coprs/forms.py

file modified

		`@@ -774,3 +774,12 @@`
		`self.errors["profiles"] = ["Missing profile name"]`
		`return False`
		`return True`
		`+`
		`+`
		`+ class ModuleRepo(wtf.Form):`
		`+ owner = wtforms.StringField("Owner Name", validators=[wtforms.validators.DataRequired()])`
		`+ copr = wtforms.StringField("Copr Name", validators=[wtforms.validators.DataRequired()])`
		`+ name = wtforms.StringField("Name", validators=[wtforms.validators.DataRequired()])`
		`+ version = wtforms.StringField("Version", validators=[wtforms.validators.DataRequired()])`
		`+ release = wtforms.StringField("Release", validators=[wtforms.validators.DataRequired()])`
		`+ arch = wtforms.StringField("Arch", validators=[wtforms.validators.DataRequired()])`

frontend/coprs_frontend/coprs/logic/complex_logic.py

file modified

		`@@ -91,6 +91,12 @@`
		`.format(user_name, copr_name))`

		`@staticmethod`
		`+ def get_copr_by_owner_safe(owner_name, copr_name, **kwargs):`
		`+ if owner_name[0] == "@":`
		`+ return ComplexLogic.get_group_copr_safe(owner_name[1:], copr_name, **kwargs)`
		`+ return ComplexLogic.get_copr_safe(owner_name, copr_name, **kwargs)`
		`+`
		`+ @staticmethod`
		`def get_copr_by_id_safe(copr_id):`
		`try:`
		`return CoprsLogic.get_by_id(copr_id).one()`

frontend/coprs_frontend/coprs/models.py

file modified

+6 -4

		`@@ -343,10 +343,6 @@`
		`self.full_name])`

		`@property`
		`- def modules_url(self):`
		`- return "/".join([self.repo_url, "modules"])`
		`-`
		`- @property`
		`def repo_id(self):`
		`if self.is_a_group_project:`
		`return "group_{}-{}".format(self.group.name, self.name)`
		`@@ -1157,3 +1153,9 @@`
		`@property`
		`def action(self):`
		`return Action.query.filter(Action.object_type == "module").filter(Action.object_id == self.id).first()`
		`+`
		`+ def repo_url(self, arch):`
		`+ # @TODO Get rid of OS name from module path, see how koji does it`
		`+ # https://kojipkgs.stg.fedoraproject.org/repos/module-base-runtime-0.25-9/latest/x86_64/toplink/packages/module-build-macros/0.1/`
		`+ module_dir = "fedora-rawhide-{}+{}-{}-{}".format(arch, self.name, self.version, self.release)`
		`+ return "/".join([self.copr.repo_url, "modules", module_dir, "latest", arch])`

frontend/coprs_frontend/coprs/views/api_ns/api_general.py

file modified

+15

		`@@ -902,3 +902,18 @@`
		`raise LegacyApiError('Chroot not found.')`

		`return flask.jsonify(output)`
		`+`
		`+`
		`+ @api_ns.route("/module/repo/", methods=["POST"])`
		`+ def copr_module_repo():`
		`+ """`
		`+ :return: URL to a DNF repository for the module`
		`+ """`
		`+ form = forms.ModuleRepo(csrf_enabled=False)`
		`+ if not form.validate_on_submit():`
		`+ raise LegacyApiError(form.errors)`
		`+`
		`+ copr = ComplexLogic.get_copr_by_owner_safe(form.owner.data, form.copr.data)`
		`+ module = ModulesLogic.get_by_nvr(copr, form.name.data, form.version.data, form.release.data).first()`
		`+`
		`+ return flask.jsonify({"output": "ok", "repo": module.repo_url(form.arch.data)})`

frontend/coprs_frontend/tests/coprs_test_case.py

file modified

+11

		`@@ -385,6 +385,17 @@`
		`created_on=int(time.time()))`
		`self.db.session.add_all([self.a1, self.a2, self.a3])`

		`+ @pytest.fixture`
		`+ def f_modules(self):`
		`+ self.m1 = models.Module(name="first-module", version="1", release="1", copr_id=self.c1.id, copr=self.c1,`
		`+ summary="Sum 1", description="Desc 1", created_on=time.time())`
		`+ self.m1 = models.Module(name="second-module", version="strver", release="3", copr_id=self.c1.id, copr=self.c1,`
		`+ summary="Sum 2", description="Desc 2", created_on=time.time())`
		`+ self.m1 = models.Module(name="third-module", version="3", release="1", copr_id=self.c2.id, copr=self.c2,`
		`+ summary="Sum 3", description="Desc 3", created_on=time.time())`
		`+ self.db.session.add_all([self.m1])`
		`+`
		`+`
		`def request_rest_api_with_auth(self, url,`
		`login=None, token=None,`
		`content=None, method="GET",`

frontend/coprs_frontend/tests/test_views/test_api_ns/test_api_general.py

file modified

+22

		`@@ -101,3 +101,25 @@`
		`# self.db.session.add_all([self.u1, self.mc1])`
		`#`
		`#`
		`+`
		`+`
		`+ class TestModuleRepo(CoprsTestCase):`
		`+ endpoint = "/api/module/repo/"`
		`+`
		`+ def test_api_module_repo(self, f_users, f_coprs, f_modules, f_db):`
		`+ data = {"owner": self.u1.name, "copr": self.c1.name, "name": "first-module",`
		`+ "version": "1", "release": "1", "arch": "x86_64"}`
		`+`
		`+ r = self.tc.post(self.endpoint, data=data)`
		`+ response = json.loads(r.data.decode("utf-8"))`
		`+ assert response["output"] == "ok"`
		`+ assert response["repo"] == "http://copr-be-dev.cloud.fedoraproject.org/results/user1/foocopr/modules/"\`
		`+ "fedora-rawhide-x86_64+first-module-1-1/latest/x86_64"`
		`+`
		`+ def test_api_module_repo_no_params(self):`
		`+ error = "This field is required."`
		`+ r = self.tc.post(self.endpoint, data={})`
		`+ response = json.loads(r.data.decode("utf-8"))`
		`+ assert response["output"] == "notok"`
		`+ for key in ["owner", "copr", "name", "version", "release", "arch"]:`
		`+ assert error in response["error"][key]`

python/copr/client/client.py

file modified

+33

		`@@ -1344,3 +1344,36 @@`
		`]`
		`)`
		`return response`
		`+`
		`+ def get_module_repo(self, owner, copr, name, version, release, arch):`
		`+ """ Gets URL to module DNF repository`
		`+`
		`+ :param owner: str owner name (can be user or @group)`
		`+ :param copr: str copr name`
		`+ :param name: str module name`
		`+ :param version: str module version`
		`+ :param release: str module release`
		`+ :param arch: str build architecture`
		`+`
		+ :return: :py:class:`~.responses.CoprResponse`
		`+ with additional fields:`
		`+`
		+ - handle: :py:class:`~.responses.BaseHandle`
		`+ - text fields: "repo"`
		`+`
		`+ """`
		`+ url = "{}/module/repo/".format(self.api_url)`
		`+ data = {"owner": owner, "copr": copr, "name": name, "version": version, "release": release, "arch": arch}`
		`+`
		`+ fetch = self._fetch(url, data=data, skip_auth=True, method="post")`
		`+ response = CoprResponse(`
		`+ client=self,`
		`+ method="get_module_repo",`
		`+ data=fetch,`
		`+ parsers=[`
		`+ CommonMsgErrorOutParser,`
		`+ ProjectListParser`
		`+ ]`
		`+ )`
		`+ response.handle = BaseHandle(client=self, response=response)`
		`+ return response`

no initial comment

frostyx commented 7 years ago

Copr moved from GitHub to Pagure during this pull request. Please see the previous conversation here https://github.com/fedora-copr/copr/pull/25

frostyx commented 7 years ago

The url classification for modules should be owner/project/nvr. That's how it is done. We can also support owner/nvr but that by returning list of module repos and let a client cope with it.

Well, I don't think that project should somehow figure in the URL or querying for the module. As I see it, modules should be identified only owner/nvr, so for instance you would use DNF like dnf module enable user/name-version-release without specifying project. Maybe, can @asamalik judge this?

Also, module repo should be a property (read @property :)) of a module and returned as such but having this method for the time being is quite fine by me.

That is a good point, see, I've already prepared ModulesLogic.get_by_nvr(...) in #2 . So after merging that one, it will be only a little change.

clime commented 7 years ago

Well, I don't think that project should somehow figure in the URL or querying for the module. As I see it, modules should be identified only owner/nvr, so for instance you would use DNF like dnf module enable user/name-version-release without specifying project. Maybe, can @asamalik judge this?

Well, modules come from projects so project certainly should figure in the full url classification. You can also see it by the added redundancy on the Module table. group_id and user_id will always be the same as for the project in the copr table, therefore, there is no point to duplicate them. owner/nvr should be a shortcut classification.