PR#128: update_sync: fix Bodhi queries when F33M release doesn't exist - fedora-qa/blockerbugs

fedora-qa / blockerbugs

#128 update_sync: fix Bodhi queries when F33M release doesn't exist

Merged 3 years ago by kparal. Opened 3 years ago by kparal.

bodhireleases into develop

update_sync: fix Bodhi queries when F33M release doesn't exist

Kamil Páral • 3 years ago

bf85b54

blockerbugs/util/update_sync.py

file modified

+33 -14

		`@@ -23,7 +23,7 @@`
		`import logging`

		`from fedora.client import ServerError`
		`- from bodhi.client.bindings import BodhiClient, BodhiClientException`
		`+ from bodhi.client.bindings import BodhiClient`

		`from blockerbugs import app`
		`from blockerbugs.models.update import Update`
		`@@ -41,6 +41,21 @@`
		`# disable saving session on disk by cache_session=False`
		`self.bodhi_interface = BodhiClient(base_url=bodhi_baseurl, cache_session=False)`
		`self.log = logging.getLogger('update_sync')`
		`+ self._releases = [] # all known releases`
		`+`
		`+ @property`
		`+ def releases(self):`
		`+ '''All releases known to Bodhi, as retrieved from /releases/ endpoint'''`
		`+ if self._releases:`
		`+ # already retrieved`
		`+ return self._releases`
		`+`
		`+ self._releases = self.bodhi_interface.get_releases(`
		`+ rows_per_page=100)['releases']`
		`+`
		`+ self.log.debug('Retrieved %d known releases from Bodhi' %`
		`+ (len(self._releases)))`
		`+ return self._releases`

		`def extract_information(self, update):`
		`updateinfo = {}`
		`@@ -114,23 +129,27 @@`
		`self.db.session.commit()`

		`def search_updates(self, bugids, release_num):`
		`+ # not all releases exist all the time (before branching, before bodhi`
		`+ # activation point, etc), so drop those which Bodhi doesn't currently`
		`+ # know of`
		`+ known_releases = [rel['name'].lower() for rel in self.releases]`
		`+ query_releases = [`
		`+ 'f%d' % release_num, # standard repo`
		`+ 'f%df' % release_num, # flatpaks`
		`+ 'f%dm' % release_num, # modularity`
		`+ ]`
		`+ for rel in query_releases.copy():`
		`+ if rel not in known_releases:`
		`+ self.log.warning("Release %s not found in Bodhi (might be "`
		`+ "normal depending on release life cycle)" % rel)`
		`+ query_releases.remove(rel)`
		`+`
		`queries_data = dict(`
		`bugs=[str(bug_id) for bug_id in bugids],`
		`- release=['f%d' % release_num, 'f%df' % release_num, 'f%dm' % release_num],`
		`+ release=query_releases,`
		`limit=100,`
		`)`
		`- try:`
		`- result = self.bodhi_interface.query(**queries_data)`
		`- except BodhiClientException as e:`
		`- # ignore invalid release error so it doesn't blow up when`
		`- # bug trackers for a release not present in bodhi are added`
		`- if 'Invalid releases' in str(e):`
		`- self.log.info("Release %d not found by Bodhi (normal if "`
		`- "release not branched yet)" % release_num)`
		`- self.log.debug(e)`
		`- return []`
		`- else:`
		`- raise`
		`+ result = self.bodhi_interface.query(**queries_data)`

		`if u'status' in result.keys():`
		`raise ServerError('', 200, result['errors'][0].description)`

testing/testfunc_update_sync.py

file modified

+14 -2

		`@@ -60,6 +60,11 @@`


		`class FakeBodhiInterface(object):`
		`+ releases = [`
		`+ {'name': 'F99'},`
		`+ {'name': 'F99F'},`
		`+ ]`
		`+`
		`updates = {`
		`('f99', '2000'): [update_for_bugs_2000_2001],`
		`('f99', '2001'): [update_for_bugs_2000_2001],`
		`@@ -74,6 +79,13 @@`
		`update2_for_bug_3000.title: datetime(2003, 1, 1),`
		`}`

		`+ def get_releases(self, **kwargs):`
		`+ return Munch(releases=self.releases,`
		`+ page=1,`
		`+ pages=1,`
		`+ rows_per_page=20,`
		`+ total=len(self.releases))`
		`+`
		`def query(self, release=None, bugs=None, **kwargs):`
		`updates = list(chain(*(self.updates.get((rel, bug), []) for bug in bugs for rel in release)))`
		`return Munch(updates=updates,`
		`@@ -82,7 +94,7 @@`
		`rows_per_page=20,`
		`total=len(updates))`

		`- class FakeErrorBodhiInterface(object):`
		`+ class FakeErrorBodhiInterface(FakeBodhiInterface):`
		`def query(self, **kwargs):`
		`return Munch(errors=[Munch(location=u"querystring",`
		`name=u"release",`
		`@@ -90,7 +102,7 @@`
		`status=u"error")`


		`- class FakeRaisesBodhiInterface(object):`
		`+ class FakeRaisesBodhiInterface(FakeBodhiInterface):`
		`def query(self, **kwargs):`
		`raise ServerError('https://admin.fedoraproject.org/updates/list',`
		`500,`

kparal commented 3 years ago

Not all releases exist all the time. Currently F33 and F33C releases exist, but
F33M doesn't. With the existing code, that completely breaks all F33 update
querying. So instead, retreive the list of releases known to Bodhi and only
skip those which are not known, instead of all-or-nothing approach. F33 updates
now correctly show up with this patch.

kparal commented 3 years ago

Btw, F33M release might get created tomorrow (according to mboddu), so don't be surprised if you see it when reviewing this. But at this very moment, it doesn't exist, and we've had the updates querying broken all along.

jskladan commented on line 55 of blockerbugs/util/update_sync.py 3 years ago

How about this instead:

        self._releases = []  # all known releases

...

    @property
    def releases(self):
        if self._releases:
            # already retrieved
            return self._releases

        self._releases = self.bodhi_interface.get_releases(
            rows_per_page=100)['releases']

        self.log.debug('Retrieved %d known releases from Bodhi' %
            (len(self._releases)))
        return self._releases

Then you could ditch the un-pythonic self.retrieve_releases() call, and just use the self.releases directly.

jskladan commented 3 years ago

Other than the, let's be honest, nitpick, LGTM. On the other hand, I can only talk about the code at hand, I have not studied the underlyling code and intent enough to be able to judge the intent :)

rebased onto bf85b54

3 years ago

kparal commented 3 years ago

Reworked according to @jskladan

kparal commented 3 years ago

Unfortunately the blocker with the update disappeared from the list, it's probably pushed, so you can't see the difference now. But the difference is seen in the log.

Before:

[connectionpool.py:939] 2020-08-25 12:10:44 DEBUG   Starting new HTTPS connection (1): bodhi.fedoraproject.org:443
[connectionpool.py:433] 2020-08-25 12:10:45 DEBUG   https://bodhi.fedoraproject.org:443 "GET /updates/?bugs=1825067&bugs=1849430&bugs=1861700&bugs=1849431&bugs=1554010&bugs=1827915&bugs=1860616&bugs=1863041&bugs=1869892&bugs=1815392&bugs=1872068&bugs=1872056&bugs=1871990&bugs=1871986&bugs=1818807&bugs=1829079&bugs=1818807&bugs=1869140&rows_per_page=100&releases=f33&releases=f33f&releases=f33m HTTP/1.1" 400 131
[update_sync.py:128] 2020-08-25 12:10:45 INFO    Release 33 not found by Bodhi (normal if release not branched yet)
[update_sync.py:130] 2020-08-25 12:10:45 DEBUG   Invalid releases specified: f33m
[update_sync.py:194] 2020-08-25 12:10:45 INFO    found 0 bugs in f34 release
[update_sync.py:197] 2020-08-25 12:10:45 INFO    no bugs in f34 release, skip update

After:

[connectionpool.py:939] 2020-08-25 12:11:12 DEBUG   Starting new HTTPS connection (1): bodhi.fedoraproject.org:443
[connectionpool.py:433] 2020-08-25 12:11:13 DEBUG   https://bodhi.fedoraproject.org:443 "GET /releases/?rows_per_page=100 HTTP/1.1" 200 25047
[update_sync.py:56] 2020-08-25 12:11:13 DEBUG   Retrieved 37 known releases from Bodhi
[update_sync.py:143] 2020-08-25 12:11:13 WARNING Release f33m not found in Bodhi (might be normal depending on release life cycle)
[connectionpool.py:433] 2020-08-25 12:11:14 DEBUG   https://bodhi.fedoraproject.org:443 "GET /updates/?bugs=1825067&bugs=1849430&bugs=1861700&bugs=1849431&bugs=1554010&bugs=1827915&bugs=1860616&bugs=1863041&bugs=1869892&bugs=1815392&bugs=1872068&bugs=1872056&bugs=1871990&bugs=1871986&bugs=1818807&bugs=1829079&bugs=1818807&bugs=1869140&rows_per_page=100&releases=f33&releases=f33f HTTP/1.1" 200 152
[update_sync.py:168] 2020-08-25 12:11:14 INFO    found 0 updates from bodhi for 18 bugs in f33
[update_sync.py:213] 2020-08-25 12:11:14 INFO    found 0 bugs in f34 release
[update_sync.py:216] 2020-08-25 12:11:14 INFO    no bugs in f34 release, skip update

The important sentence is:

found 0 updates from bodhi for 18 bugs in f33

which is only present in After. Because in Before, the Invalid releases specified: f33m error killed the whole process for F33.

kparal commented 3 years ago

@lbrabec @frantisekz wdyt?

lbrabec commented 3 years ago

:thumbsup:

frantisekz commented 3 years ago

lgtm

kparal commented 3 years ago

Commit cdeabed fixes this pull-request

Pull-Request has been merged by kparal

3 years ago

Metadata

Assignee

None

Tags

No Tags

Changes Summary 2

+33 -14

file changed

blockerbugs/util/update_sync.py

+14 -2

file changed

testing/testfunc_update_sync.py

fedora-qa / blockerbugs

Source Code

#128 update_sync: fix Bodhi queries when F33M release doesn't exist Merged 3 years ago by kparal. Opened 3 years ago by kparal. bodhireleases into develop

Metadata

Changes Summary 2

#128 update_sync: fix Bodhi queries when F33M release doesn't exist

Merged 3 years ago by kparal. Opened 3 years ago by kparal.

bodhireleases into develop