PR#1130: cli, python, frontend: add command to list all available chroots - copr/copr

copr / copr

#1130 cli, python, frontend: add command to list all available chroots

Merged 5 years ago by praiskup. Opened 5 years ago by dturecek.

copr/ dturecek/copr chroot_list into master

cli, python, frontend: add command to list all available chroots

Dominik Turecek • 5 years ago

1a2fe80

cli/copr_cli/main.py

file modified

+14

		`@@ -11,6 +11,7 @@`
		`import simplejson`
		`import requests`
		`from collections import defaultdict`
		`+ from textwrap import indent`

		`import logging`
		`if six.PY2:`
		`@@ -534,6 +535,16 @@`
		`)`
		`print(json_dumps(project_chroot))`

		`+ def action_list_chroots(self, args):`
		`+ """List all currently available chroots.`
		`+ """`
		`+ chroots = self.client.mock_chroot_proxy.get_list()`
		`+ chroots = simplejson.loads(json_dumps(chroots))`
		`+ for chroot, comment in chroots.items():`
		`+ print(chroot)`
		`+ if comment:`
		`+ print(indent(comment, ' '))`
		`+`
		`#########################################################`
		`### Package actions ###`
		`#########################################################`
		`@@ -1050,6 +1061,9 @@`
		`parser_get_chroot.add_argument("coprchroot", help="Path to a project chroot as owner/project/chroot or project/chroot")`
		`parser_get_chroot.set_defaults(func="action_get_chroot")`

		`+ parser_list_chroots = subparsers.add_parser("list-chroots", help="List all currently available chroots.")`
		`+ parser_list_chroots.set_defaults(func="action_list_chroots")`
		`+`
		`#########################################################`
		`### Package options ###`
		`#########################################################`

cli/tests/test_cli.py

file modified

+12

		`@@ -681,3 +681,15 @@`
		`test_me( # we don't parse '=' here`
		`['some/project', '--admin', 'bad_status=nothing'],`
		`{'your user': {'admin': 'bad_status=nothing'}} )`
		`+`
		`+`
		`+ @mock.patch('copr.v3.proxies.mock_chroot.MockChrootProxy.get_list')`
		`+ def test_list_chroots(list_chroots):`
		`+ list_chroots.return_value = Munch({`
		`+ "fedora-18-x86_64": "",`
		`+ "fedora-17-x86_64": "A short chroot comment",`
		`+ "fedora-17-i386": "Chroot comment containing [url with four\nwords](https://copr.fedorainfracloud.org/)",`
		`+ "fedora-rawhide-i386": "",`
		`+ })`
		`+`
		`+ main.main(argv=["list-chroots"])`

frontend/copr-frontend.spec

file modified

		`@@ -85,6 +85,7 @@`
		`BuildRequires: python3-flask-whooshee`
		`BuildRequires: python3-flask-wtf`
		`BuildRequires: python3-gobject`
		`+ BuildRequires: python3-html2text`
		`BuildRequires: python3-html5-parser`
		`BuildRequires: python3-humanize`
		`BuildRequires: python3-lxml`
		`@@ -138,6 +139,7 @@`
		`Requires: python3-flask-wtf`
		`Requires: python3-flask-wtf`
		`Requires: python3-gobject`
		`+ Requires: python3-html2text`
		`Requires: python3-html5-parser`
		`Requires: python3-humanize`
		`Requires: python3-lxml`

frontend/coprs_frontend/coprs/__init__.py

file modified

+1 -1

		`@@ -63,7 +63,7 @@`
		`from coprs.views.api_ns import api_general`
		`from coprs.views import apiv3_ns`
		`from coprs.views.apiv3_ns import (apiv3_general, apiv3_builds, apiv3_packages, apiv3_projects, apiv3_project_chroots,`
		`- apiv3_modules, apiv3_build_chroots,`
		`+ apiv3_modules, apiv3_build_chroots, apiv3_mock_chroots,`
		`apiv3_permissions)`
		`from coprs.views import coprs_ns`
		`from coprs.views.coprs_ns import coprs_builds`

frontend/coprs_frontend/coprs/logic/coprs_logic.py

file modified

		`@@ -806,6 +806,10 @@`
		`return [ch.name for ch in cls.get_multiple(active_only=True).all()]`

		`@classmethod`
		`+ def active_names_with_comments(cls):`
		`+ return [(ch.name, ch.comment) for ch in cls.get_multiple(active_only=True).all()]`
		`+`
		`+ @classmethod`
		`def new(cls, mock_chroot):`
		`db.session.add(mock_chroot)`

frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_mock_chroots.py

file added

+17

		`@@ -0,0 +1,17 @@`
		`+ import flask`
		`+ from html2text import html2text`
		`+ from coprs.views.apiv3_ns import apiv3_ns`
		`+ from coprs.logic.coprs_logic import MockChrootsLogic`
		`+`
		`+`
		`+ @apiv3_ns.route("/mock-chroots/list")`
		`+ def list_chroots():`
		`+ chroots = MockChrootsLogic.active_names_with_comments()`
		`+ response = {}`
		`+ for chroot, comment in chroots:`
		`+ if comment:`
		`+ response[chroot] = html2text(comment).strip("\n")`
		`+ else:`
		`+ response[chroot] = ""`
		`+`
		`+ return flask.jsonify(response)`

frontend/coprs_frontend/tests/coprs_test_case.py

file modified

+4 -2

		`@@ -151,11 +151,13 @@`
		`self.mc1.distgit_branch = models.DistGitBranch(name='f18')`

		`self.mc2 = models.MockChroot(`
		`- os_release="fedora", os_version="17", arch="x86_64", is_active=True)`
		`+ os_release="fedora", os_version="17", arch="x86_64", is_active=True,`
		`+ comment="A short chroot comment")`
		`self.mc2.distgit_branch = models.DistGitBranch(name='fedora-17')`

		`self.mc3 = models.MockChroot(`
		`- os_release="fedora", os_version="17", arch="i386", is_active=True)`
		`+ os_release="fedora", os_version="17", arch="i386", is_active=True,`
		`+ comment="Chroot comment containing <a href='https://copr.fedorainfracloud.org/'>url with four words</a>")`
		`self.mc3.distgit_branch = self.mc2.distgit_branch`

		`self.mc4 = models.MockChroot(`

frontend/coprs_frontend/tests/test_apiv3/test_mock_chroot.py

file added

+15

		`@@ -0,0 +1,15 @@`
		`+ import json`
		`+ from tests.coprs_test_case import CoprsTestCase`
		`+`
		`+`
		`+ class TestMockChroot(CoprsTestCase):`
		`+`
		`+ def test_list_available_chroots(self, f_mock_chroots):`
		`+ r = self.tc.get("/api_3/mock-chroots/list")`
		`+ assert r.status_code == 200`
		`+ assert json.loads(r.data) == {`
		`+ "fedora-18-x86_64": "",`
		`+ "fedora-17-x86_64": "A short chroot comment",`
		`+ "fedora-17-i386": "Chroot comment containing [url with four\nwords](https://copr.fedorainfracloud.org/)",`
		`+ "fedora-rawhide-i386": "",`
		`+ }`

python/copr/v3/__init__.py

file modified

		`@@ -6,6 +6,7 @@`
		`from .proxies.project import ProjectProxy`
		`from .proxies.build import BuildProxy`
		`from .proxies.package import PackageProxy`
		`+ from .proxies.mock_chroot import MockChrootProxy`
		`from .proxies.project_chroot import ProjectChrootProxy`
		`from .proxies.build_chroot import BuildChrootProxy`
		`from .proxies.module import ModuleProxy`
		`@@ -24,6 +25,7 @@`
		`BaseProxy,`
		`BuildProxy,`
		`PackageProxy,`
		`+ MockChrootProxy,`
		`ProjectChrootProxy,`
		`BuildChrootProxy,`
		`ModuleProxy,`

python/copr/v3/client.py

file modified

		`@@ -4,6 +4,7 @@`
		`from .proxies.build import BuildProxy`
		`from .proxies.package import PackageProxy`
		`from .proxies.module import ModuleProxy`
		`+ from .proxies.mock_chroot import MockChrootProxy`
		`from .proxies.project_chroot import ProjectChrootProxy`
		`from .proxies.build_chroot import BuildChrootProxy`

		`@@ -16,6 +17,7 @@`
		`self.build_proxy = BuildProxy(config)`
		`self.package_proxy = PackageProxy(config)`
		`self.module_proxy = ModuleProxy(config)`
		`+ self.mock_chroot_proxy = MockChrootProxy(config)`
		`self.project_chroot_proxy = ProjectChrootProxy(config)`
		`self.build_chroot_proxy = BuildChrootProxy(config)`

python/copr/v3/proxies/mock_chroot.py

file added

+18

		`@@ -0,0 +1,18 @@`
		`+ from __future__ import absolute_import`
		`+`
		`+ import os`
		`+ from . import BaseProxy`
		`+ from ..requests import Request, munchify`
		`+`
		`+`
		`+ class MockChrootProxy(BaseProxy):`
		`+`
		`+ def get_list(self, pagination=None):`
		`+ """List all currently available chroots.`
		`+`
		`+ :return: Munch`
		`+ """`
		`+ endpoint = "/mock-chroots/list"`
		`+ request = Request(endpoint, api_base_url=self.api_base_url)`
		`+ response = request.send()`
		`+ return munchify(response)`

dturecek commented 5 years ago

Fixes: #1117

dturecek commented 5 years ago

Example:

# copr-cli list-chroots
centos-stream-x86_64
custom-1-i386
custom-1-ppc64le
custom-1-x86_64
epel-6-i386
epel-6-x86_64
epel-7-aarch64
epel-7-ppc64le
epel-7-x86_64
epel-8-aarch64
epel-8-ppc64le
epel-8-x86_64
fedora-29-aarch64
fedora-29-i386
fedora-29-ppc64le
fedora-30-aarch64
fedora-30-armhfp: This is emulated on x86_64
fedora-30-i386
fedora-30-ppc64le
fedora-30-x86_64
fedora-31-aarch64
fedora-31-armhfp: This is emulated on x86_64
fedora-31-i386: Not-released Koji packages, [more
info](https://fedoraproject.org/wiki/Changes/Noi686Repositories)
fedora-31-ppc64le
fedora-31-x86_64
fedora-32-i386
fedora-32-x86_64
fedora-rawhide-aarch64
fedora-rawhide-armhfp: This is emulated on x86_64
fedora-rawhide-i386: Not-released Koji packages, [more
info](https://fedoraproject.org/wiki/Changes/Noi686Repositories)
fedora-rawhide-ppc64le
fedora-rawhide-x86_64
mageia-6-i586
mageia-6-x86_64
mageia-cauldron-i586
mageia-cauldron-x86_64
opensuse-leap-15.0-x86_64
opensuse-tumbleweed-i586
opensuse-tumbleweed-ppc64le
opensuse-tumbleweed-x86_64

frostyx commented 5 years ago

It looks good, thank you.
However, I would prefer not having it in apiv3_general.py and BaseProxy, but rather somewhere else. We have apiv3_build_chroots.py and apiv3_project_chroots.py, so I would go with creating apiv3_mock_chroots.py.

It will represent our internal structure the best, but if you think it might be confusing for users (what's the difference between build chroot, project chroot and mock chroot), then we can possibly move it into some of the existing files (e.g. /project-chroots/list-available or something like this).

dturecek commented 5 years ago

so I would go with creating apiv3_mock_chroots.py

This makes sense, I'll do it this way.

praiskup commented 5 years ago

What if we had this (yaml) format:

fedora-31-aarch64: >
fedora-31-armhfp: >
        This is emulated on x86_64
fedora-31-i386: >
        Not-released Koji packages, [more
        info](https://fedoraproject.org/wiki/Changes/Noi686Repositories)

... so people could just json.load(the_text) to have a valid dict?

praiskup commented 5 years ago

I don't know, maybe we don't need parse-able text on output; then we could
avoid the : > part of the lines:

fedora-31-aarch64
fedora-31-armhfp
        This is emulated on x86_64
fedora-31-i386
        Not-released Koji packages, [more
        info](https://fedoraproject.org/wiki/Changes/Noi686Repositories)

frostyx commented 5 years ago

... so people could just json.load(the_text) to have a valid dict?

I was also thinking about this and ... well, you won't need to use json.load(text) on the output because in that case, it makes much more sense to just use python-copr package instead and make the API call by yourself. But I think that it is a good idea to have it somehow easily parseable. You might want to use | jq on the output or something. Or you might want to print just the list of names without descriptions.

praiskup commented 5 years ago

s/jq/yq/ and s/json.loads/yaml.load/

because in that case, it makes much more sense to just use python-copr package

I agree that it would make much more sense, but not that it is that trivial; and
some people don't work with python. But yes, let's concentrate on readability
ATM.

frostyx commented 5 years ago

I meant that parsing with the json.load(text) as you suggested above is unnecessary because if you know python, you will use our python library. But I think, that parsing with other command-line tools is a valid use-case.

rebased onto 612cc0fc26a0f1e775ae6c56b7c9a7e493b8ab71

5 years ago

dturecek commented 5 years ago

I've moved the code from BaseProxy to MockChrootProxy and changed the format of output to:

fedora-31-aarch64
fedora-31-armhfp
    This is emulated on x86_64
fedora-31-i386
    Not-released Koji packages, [more
    info](https://fedoraproject.org/wiki/Changes/Noi686Repositories)

frostyx commented on line 15 of frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_general.py 5 years ago

This is still in apiv3_general.py. Also after moving it into (possibly) apiv3_mock_chroots.py, i would change the URL to simply /list to be consistent with other routes.

praiskup commented on line 13 of cli/copr-cli.spec 5 years ago

Can you please move this dependancy to frontend, and make frontend send the filtered data to client?

praiskup commented 5 years ago

Also please add a test cases, especially including some html text in some chroot description.

rebased onto 25e7a65012b7aac91323e49061297c83ac4943e6

5 years ago

rebased onto 2d54469b51730998e0ac6d46537685fbeedcc71b

5 years ago

rebased onto 66d81271cb97db86c58fb5e3d5f420b953ce1a6d

5 years ago

rebased onto 8e35d8dbd5ef3e465644a348a244e3bda15dec01

5 years ago

rebased onto 7be728b7fe6c839081a79179df238f353e2c0413

5 years ago

dturecek commented 5 years ago

Done, please take another look.

praiskup commented 5 years ago

[copr-build]

rebased onto b7e061550ba995510eab71d873c654d2f88d3c87

5 years ago

dturecek commented 5 years ago

I've changed the test a bit to contain a comment with multiple words between link tags.

praiskup commented 5 years ago

rebased onto 60f67e17ca4a4b0d5509cab33e26054810683881

5 years ago

praiskup commented 5 years ago

Hm, ...

$ copr list-chroots
fedora-31-aarch64
fedora-31-armhfp
Traceback (most recent call last):
  File "/usr/lib64/python3.7/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/usr/lib64/python3.7/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/home/praiskup/rh/projects/copr/copr/cli/copr_cli/main.py", line 1364, in <module>
    main()
  File "/home/praiskup/rh/projects/copr/copr/cli/copr_cli/main.py", line 1336, in main
    getattr(commands, arg.func)(arg)
  File "/home/praiskup/rh/projects/copr/copr/cli/copr_cli/main.py", line 545, in action_list_chroots
    print(indent(comment, '    '))
NameError: name 'indent' is not defined

Please add simple test-case which would catch this into client testsuite.

rebased onto 3c8f17144c42d27aab97062ee93e09e99aaa9b08

5 years ago

dturecek commented 5 years ago

Test added.

frostyx commented on line 11 of python/copr/v3/proxies/mock_chroot.py 5 years ago

To be consistent also here, with the rest of the proxy classes, I would go with get_list(self, pagination=None)

frostyx commented 5 years ago

Except for that one note, this PR looks really good, thank you!

rebased onto 3ea3c3b10970b6640f2ebb262a3f21fbdb822852

5 years ago

praiskup commented 5 years ago

[copr-build]

rebased onto 1a2fe80

5 years ago

Pull-Request has been merged by praiskup

5 years ago

Metadata

Assignee

None

Tags

No Tags

copr / copr

Source Code

Documentation

#1130 cli, python, frontend: add command to list all available chroots Merged 5 years ago by praiskup. Opened 5 years ago by dturecek. copr/ dturecek/copr chroot_list into master

Metadata

Flags

Changes Summary 11

#1130 cli, python, frontend: add command to list all available chroots

Merged 5 years ago by praiskup. Opened 5 years ago by dturecek.

copr/ dturecek/copr chroot_list into master