Skip to content
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

Add some cosmetics to identify the ReviewRequest ID #17

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -9,6 +9,10 @@ only-test:
checkstyle:
black --check ./

.PHONY: tidy-diff
tidy-diff:
black --diff ./

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

valid extension. But please make that a separate commit

.PHONY: tidy
tidy:
black ./
6 changes: 5 additions & 1 deletion mtui/main.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import logging
import sys
from subprocess import CalledProcessError
import setproctitle
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, nonexisting module


from mtui.args import get_parser
from mtui.config import Config
@@ -11,6 +12,7 @@

from .argparse import ArgsParseFailure
from .colorlog import create_logger
from .utils import get_short_rrid
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why special function if this can be method of RequestReviewID class ?



def main():
@@ -33,7 +35,6 @@ def main():


def run_mtui(sys, config, logger, Prompt, Display, args):

if args.debug:
logger.setLevel(level=logging.DEBUG)

@@ -45,6 +46,9 @@ def run_mtui(sys, config, logger, Prompt, Display, args):

prompt = Prompt(config, logger, sys, Display)
if args.update:
short_rrid = get_short_rrid(str(args.update.id))
# helps to set window/tab title with reviewid
setproctitle.setproctitle("mtui-" + short_rrid)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw how will load_template command change title ?

if args.update.kind == "kernel":
config.kernel = True
config.auto = False
1 change: 1 addition & 0 deletions mtui/parsemetajson.py
Original file line number Diff line number Diff line change
@@ -12,6 +12,7 @@ def parse(results, data):

results.rrid = RequestReviewID(data.get("rrid"))
results.packager = data.get("packager")
results.srpms = data.get("SRCRPMs")
results.rating = data.get("rating")
results.repository = data.get("repository")
results.category = data.get("category")
8 changes: 5 additions & 3 deletions mtui/prompt.py
Original file line number Diff line number Diff line change
@@ -14,6 +14,7 @@
from . import commands, messages
from .argparse import ArgsParseFailure
from .template.nulltestreport import NullTestReport
from .utils import get_short_rrid

logger = getLogger("mtui.prompt")

@@ -221,16 +222,17 @@ def emptyline(self):

def set_prompt(self, session=None):
self.session = session
session = ":" + str(session) if session else ""
session = str(session) + "/" if session else ""
mode = "mtui"
if self.config.auto and not self.config.kernel:
mode += "-auto"
elif self.config.kernel:
mode += "-kernel"
self.prompt = f"{mode}{session}> "
self.prompt = f"{session}{mode}> "

def load_update(self, update, autoconnect):
tr = update.make_testreport(self.config, autoconnect=autoconnect)
self.metadata = tr
self.targets = tr.targets
self.set_prompt(None)
short_rrid = get_short_rrid(str(update.id))
self.set_prompt(short_rrid)
13 changes: 12 additions & 1 deletion mtui/template/testreport.py
Original file line number Diff line number Diff line change
@@ -85,13 +85,15 @@ def __init__(self, config, scripts_src_dir=None):
self.reviewer = ""
self.repository = None
self.packages = {}
self.srpms = []

self._attrs = [
"products",
"category",
"packager",
"reviewer",
"packages",
"srpms",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and parser for srpms is where ? and data source ?

"bugs",
"repository",
]
@@ -160,7 +162,6 @@ def _parse(self, tpl: str) -> None:
self._warn_missing_fields()

def _parse_json(self, data, tpl: str) -> None:

if self.path:
raise TestReportAlreadyLoaded(self.path)

@@ -430,6 +431,14 @@ def refhosts_from_tp(self, testplatform):
def list_bugs(self, sink, arg):
return sink(self.bugs, self.jira, arg)

def _get_result_links(self):
incident_id = self.repository.rstrip("/").split("/")[-1]
dboard_burl = "http://dashboard.qam.suse.de/incident/"
openqa_burl = "https://openqa.suse.de/tests/overview?build=:"
Comment on lines +436 to +437
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please avoid specifying any SUSE internal URLs that are not relevant to this "upstream" open source project

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So should I avoid this change entirely?
or get the base urls from some other ways like the user environment?
or have them in mtui/config.py where a few simillar urls are already defined?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reading from user environment or mtu/config.py both sounds good. I would just avoid specifying any SUSE internal URLs but for convenience maybe it makes sense to provide them but then please in a specific config file. https://github.com/openSUSE/mtui/blob/main/mtui/config.py#L114 looks similar so I guess it's ok if you define URLs there.

dboard_lnk = dboard_burl + incident_id
openqa_lnk = openqa_burl + incident_id + ":" + self.srpms[0]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rly ? this can't work

return (dboard_lnk, openqa_lnk)

def _show_yourself_data(self):
return (
[
@@ -443,6 +452,8 @@ def _show_yourself_data(self):
("Build checks", self._testreport_url()[:-3] + "build_checks"),
("Testreport", self._testreport_url()),
("Repository", self.repository),
("DashBoard Link", self._get_result_links()[0]),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
("DashBoard Link", self._get_result_links()[0]),
("Dashboard Link", self._get_result_links()[0]),

("OpenQA Link", self._get_result_links()[1]),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
("OpenQA Link", self._get_result_links()[1]),
("openQA Link", self._get_result_links()[1]),

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also lowercase "Link" here, as well as in the line above. If this ends up rendered as link then maybe also drop " Link" completely. The other entries also don't have " Link".

]
+ [("Testplatform", x) for x in self.testplatforms]
+ [("Products", x) for x in self.products]
4 changes: 4 additions & 0 deletions mtui/utils.py
Original file line number Diff line number Diff line change
@@ -333,3 +333,7 @@ def walk(inc):
if isinstance(inc[key], (list, dict)):
inc[key] = walk(inc[key])
return inc


def get_short_rrid(id_str):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why ? this should be method of RequestReviewID ...

return id_str.replace("SUSE", "S").replace("Maintenance", "M")
10 changes: 9 additions & 1 deletion tests/test_utils.py
Original file line number Diff line number Diff line change
@@ -2,6 +2,7 @@
from mtui.utils import chdir
from mtui.utils import atomic_write_file
from mtui.utils import SUTParse
from mtui.utils import get_short_rrid

import os

@@ -18,7 +19,6 @@ def create_temp(tmpdir_factory):


class TestEnsureDirExists(object):

_callback_paths = []

def test_create(self, create_temp):
@@ -83,3 +83,11 @@ def test_atomic_write(create_temp):

def test_sutparse():
pass


def test_get_short_rrid():
id = "10010:200200"
arb_rrid = "SUSE:Maintenance:" + id
short_rrid = get_short_rrid(arb_rrid)

assert short_rrid == "S:M:" + id