fedora-qa / blockerbugs

Clone
Source Code
GIT

#138 bug status as svg endpoint
Merged 3 years ago by lbrabec. Opened 3 years ago by lbrabec.
feature/bugimg  into  develop

file modified
+5 -2 
@@ -47,6 +47,8 @@ 
 

      BEHIND_PROXY = False
 

      FEDMENU_URL = ""
 

      FEDMENU_DATA_URL = ""
 

+     BLOCKERBUGS_URL = "https://qa.fedoraproject.org/blockerbugs/"
 

+     BLOCKERBUGS_API = "{}api/v0/".format(BLOCKERBUGS_URL)
 

      PAGURE_URL = "https://stg.pagure.io/"
 

      PAGURE_API = "https://stg.pagure.io/api/0/"
 

      PAGURE_REPO = "fedoraqa-test"
 
@@ -58,13 +60,14 @@ 
 

      PAGURE_DISCUSSION_TITLE = "$bugid $summary"
 

      PAGURE_DISCUSSION_CONTENT = '''\
 

  Bug details: ** $bug_url **
 

- 

+ Information from [BlockerBugs App]({}):
 

+ ![$bugid]($bug_img)
 

  #### Current vote summary
 

  $vote_summary
 

  

  To learn how to vote, see:
 

  $pagure_url$pagure_repo
 

- '''
 

+ '''.format(BLOCKERBUGS_URL)
 

  

  

  class ProductionConfig(Config):

file modified
+51 -1 
@@ -31,7 +31,7 @@ 
 

  from blockerbugs.models.bug import Bug
 

  from blockerbugs.util import pagure_bot
 

  from . import errors
 

- from .utils import get_or_404, JsonResponse, check_signature
 

+ from .utils import get_or_404, JsonResponse, SVGResponse, check_signature
 

  

  api_v0 = Blueprint('api', __name__, url_prefix='/api/v0')
 

  
@@ -143,3 +143,53 @@ 
 

          msg = 'Unable to parse issue id from received message'
 

          app.logger.debug(msg)
 

          return JsonResponse({'msg': msg})
 

+ 

+ 

+ def _svg_response_text(info_all):
 

+     svg_template = '<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" height="{total_height}" width="300">'\
 

+                    '{info_lines}'\
 

+                    '</svg>'
 

+     info_line_template = '<text font-family="sans-serif" x="0" y="{y_offset}" font-size="14px" fill="#212529">{info}</text>'
 

+ 

+     # height of each line is 17px, text is rendered at y offset 13px (4px bellow as padding)
 

+     # y_offset of nth line is 13 * (n + 1) + 4 * n = 13*n + 13 + 4*n = 17*n + 13
 

+     info_lines = [info_line_template.format(y_offset=17 * index + 13,
 

+                                             info=info) for index, info in enumerate(info_all)]
 

+     return svg_template.format(total_height=17 * len(info_all), info_lines="".join(info_lines))
 

+ 

+ 

+ def _get_bugtypes(bug):
 

+     bugtypes = []
 

+     if bug.proposed_blocker:
 

+         bugtypes.append("Proposed Blocker")
 

+     if bug.proposed_fe:
 

+         bugtypes.append("Proposed FE")
 

+     if bug.rejected_blocker:
 

+         bugtypes.append("Rejected Blocker")
 

+     if bug.rejected_fe:
 

+         bugtypes.append("Rejected FE")
 

+     if bug.accepted_blocker:
 

+         bugtypes.append("Accepted Blocker")
 

+     if bug.accepted_0day:
 

+         bugtypes.append("Accepted 0day")
 

+     if bug.accepted_prevrel:
 

+         bugtypes.append("Accepted Previous release")
 

+     if bug.accepted_fe:
 

+         bugtypes.append("Accepted FE")
 

+     return bugtypes
 

+ 

+ _UNKNOWN_BUG_SVG_TEXT = _svg_response_text(["unknown bug"])
 

+ 

+ @api_v0.route('/bugimg/<int:bug_id>')
 

+ def bug_image(bug_id):
 

+     bugs = Bug.query.filter_by(bugid=bug_id).all()
 

+     if not bugs:
 

+         return SVGResponse(_UNKNOWN_BUG_SVG_TEXT)

I'd personally get rid of the _UNKNOWN_BUG_SVG_TEXT variable, and just replace this with return SVGResponse(_svg_response_text(["unknown bug"])). The variable doesn't seem to be used elsewhere anyway, and also this way the string doesn't need to be computed each time this module is loaded. But that's just a nitpick, not important.

Edit: I missed that the variable is used in the unit test. ¯_(ツ)_/¯ Both approaches work fine here.

Edited 3 years ago by kparal 

+     else:
 

+         info_all = []
 

+         for bug in bugs:
 

+             milestone_info = "%s: " % bug.milestone.name
 

+             bugtypes = _get_bugtypes(bug)
 

+             info_all.append(milestone_info + ", ".join(bugtypes))
 

+ 

+         return SVGResponse(_svg_response_text(info_all))

file modified
+9 
@@ -75,6 +75,15 @@ 
 

          super(JsonResponse, self).__init__(data, *args, **kwargs)
 

  

  

+ class SVGResponse(Response):
 

+     default_mimetype = 'image/svg+xml'
 

+ 

+     def __init__(self, response=None, *args, **kwargs):
 

+         if response is None:
 

+             response = ""
 

+         super(SVGResponse, self).__init__(response, *args, **kwargs)
 

+ 

+ 

  def json_loads(data):
 

      try:
 

          return json.loads(data)

file modified
+4 
@@ -14,6 +14,8 @@ 
 

          bugid=bug.bugid,
 

          summary=bug.summary)
 

      content = Template(app.config['PAGURE_DISCUSSION_CONTENT']).safe_substitute(
 

+         bugid=bug.bugid,
 

+         bug_img=app.config['BLOCKERBUGS_API'] + "bugimg/%s" % bug.bugid,
 

          bug_url=bug.url,
 

          vote_summary='Nobody voted yet.',
 

          pagure_url=app.config['PAGURE_URL'],
 
@@ -33,6 +35,8 @@ 
 

          bugid=bug.bugid,
 

          summary=bug.summary)
 

      content = Template(app.config['PAGURE_DISCUSSION_CONTENT']).safe_substitute(
 

+         bugid=bug.bugid,
 

+         bug_img=app.config['BLOCKERBUGS_API'] + "bugimg/%s" % bug.bugid,
 

          bug_url=bug_url,
 

          vote_summary=vote_summary,
 

          pagure_url=app.config['PAGURE_URL'],

file modified
+1 
@@ -23,3 +23,4 @@ 
 

  PAGURE_BOT_USERNAME = 'blockerbot'
 

  SHOW_DB_URI = False
 

  BEHIND_PROXY = False
 

+ BLOCKERBUGS_API = "https://qa.fedoraproject.org/blockerbugs/api/v0/"

file modified
+23 
@@ -10,11 +10,13 @@ 
 

  

  from blockerbugs import db
 

  from blockerbugs import app
 

+ from blockerbugs.models.bug import Bug
 

  from blockerbugs.models.build import Build
 

  from blockerbugs.models.userinfo import UserInfo
 

  from testing.test_controllers import add_release, add_milestone, \
 

      add_bug, add_update
 

  from blockerbugs.controllers.api import errors
 

+ from blockerbugs.controllers.api.api import _get_bugtypes, _UNKNOWN_BUG_SVG_TEXT
 

  

  

  class TestRestAPI(object):
 
@@ -140,3 +142,24 @@ 
 

          assert resp.status_code == httplib.OK
 

          data = json.loads(resp.data)
 

          assert data['name'] == self.milestone.name
 

+ 

+     def test_get_bugimg(self):
 

+         url = '/api/v0/bugimg/9002'
 

+         resp = self.client.get(url)
 

+         assert resp.status_code == httplib.OK
 

+         data = str(resp.data)
 

+ 

+         bug2 = db.session.query(Bug).filter_by(bugid = 9002).first()
 

+ 

+         assert self.milestone.name in data
 

+         for bugtype in _get_bugtypes(bug2):
 

+             assert bugtype in data
 

+ 

+     def test_get_bugimg_wrong_bugid(self):
 

+         url = '/api/v0/bugimg/90210'
 

+         resp = self.client.get(url)
 

+         assert resp.status_code == httplib.OK
 

+         data = str(resp.data)
 

+ 

+         assert self.milestone.name not in data
 

+         assert _UNKNOWN_BUG_SVG_TEXT in data

This change adds a new endpoint /api/v0/bugimg/<bugid>. It returns svg image with current info what the bug is proposed as from blockerbugs app.
The image can be included in the initial description of discussion issue. This way, we get fresh info from blockerbugs app without updating the issue itself.

note: If you want to test this locally, I presume it will by over http and not https. You will need to disable content security policy headers for pagure to load the image.

Build succeeded.

It would make sense, IMO, to split this into something like:

BLOCKERBUGS_URL = "https://qa.fedoraproject.org/blockerbugs/"
BLOCKERBUGS_API = "{}api/v0/".format(BLOCKERBUGS_URL)

And then using the BLOCKERBUGS_URL instead of the hard-coded string below.

1 new commit added

  • use config variable instead of hardcoded string
3 years ago

Build succeeded.

Maybe width="100%" instead? I'm no SVG-maister, but from what I've read, this might play nicer in the grand scheme of things of random compatibility and rendering issues.

hm, that results in:
img

Weird, asi I said, I'm no SVG master, if the fixed 400px works good, then absolutely go for it. From what I've read about the rendering pipeline, the 100% should have worked fine (better), but...

So, this will put everything on a single line and crop the information after 400px? That seems suboptimal. I think it would look better this way:

Information from BlockerBugs App:
33-beta: Proposed FE, Proposed Blocker
33-final: Proposed Blocker

It would also decrease the chance of cropping the text (if we make the width sufficient enough). Of course the best solution would be to make the picture as wide as possible, if you can find a way. But that can come later, if we split the information by newlines.

img

1 new commit added

  • multiline svg
3 years ago

Build succeeded.

please use named fields inside templates (here and everywhere else), otherwise it's get quite hard to read it when you expand the template

a comment about these magic values (13 and 17) would be nice, I have no idea how you discovered them and what they mean

Apart from those nitpicks, the PR looks fine to me. I haven't tried it myself, because I didn't want to fight with HTTPS issues.

rebased onto 7b3d55d
3 years ago

Build succeeded.

This will now crash, right? Could you please add at least a basic unit test for a valid and an invalid pass? To avoid these errors in the future. Thanks.

rebased onto 14820ab
3 years ago

Build succeeded.

rebased onto 8d32b1b
3 years ago

Build succeeded.

I'd personally get rid of the _UNKNOWN_BUG_SVG_TEXT variable, and just replace this with return SVGResponse(_svg_response_text(["unknown bug"])). The variable doesn't seem to be used elsewhere anyway, and also this way the string doesn't need to be computed each time this module is loaded. But that's just a nitpick, not important.

Edit: I missed that the variable is used in the unit test. ¯_(ツ)_/¯ Both approaches work fine here.
Edited 3 years ago by kparal

The code looks OK to me.
Note: It would be easier to review if you added a new commit next time, so that I could see the differences. You can always squash it all before committing.

Anyone else wants to review this?

No responses, I think you can merge this @lbrabec .

I guess we can test this in production once Beta freeze is lifted?

Btw, you can edit the commit message to include:

Fixes https://pagure.io/fedora-qa/blockerbugs/issue/116
Merges https://pagure.io/fedora-qa/blockerbugs/pull-request/138

Pull-Request has been merged by lbrabec
3 years ago
Metadata
None
No Tags
Flags
Zuul
success
Jobs result is success
3 years ago
Changes Summary 6
+5 -2
file changed
blockerbugs/config.py
+51 -1
file changed
blockerbugs/controllers/api/api.py
+9 -0
file changed
blockerbugs/controllers/api/utils.py
+4 -0
file changed
blockerbugs/util/pagure_interface.py
+1 -0
file changed
conf/settings.py.example
+23 -0
file changed
testing/test_api.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.