Drop the update.title field from the database #3038

bowlofeggs · 2019-02-22T16:22:15Z

There are two commits in this pull request. Please see their messages for details. The summary:

The first one indexes a field that really should have always been indexed. This is needed for the downgrade() on the migration for the second commit, which takes more than 10 minutes without an index (I killed it, so no idea how long it actually takes, just that it's more than 10 minutes). With the index, the downgrade() takes about 7 seconds on my laptop.
The second commit drops the title field from updates, and disallows referencing updates by title.

bowlofeggs · 2019-02-22T16:22:53Z

This pull request will not pass CI until #2987 is merged, as it uses some Python 3-only syntax in the CLI.

Zlopez

LGTM

sebwoj · 2019-02-26T09:38:23Z

bodhi/server/models.py

@@ -1672,7 +1666,7 @@ class Update(Base):
    date_stable = Column(DateTime)

    # eg: FEDORA-EPEL-2009-12345
-    alias = Column(Unicode(32), unique=True, nullable=True)
+    alias = Column(Unicode(32), unique=True, nullable=False)


With this change, we can mark commit as fixing #1714

Fixed, and thanks!

sebwoj · 2019-02-26T09:44:55Z

bodhi/tests/server/test_models.py

@@ -3248,7 +3246,7 @@ def test_comment_empty(self):
        self.assertEqual(str(exc.exception), 'You must provide either some text or feedback')

    def test_get_url(self):
-        self.assertEqual(self.obj.get_url(), u'updates/TurboGears-1.0.8-3.fc11')
+        self.assertEqual(self.obj.get_url(), f'updates/{self.obj.alias}')
        idx = 'a3bbe1a8f2'
        with mock.patch(target='uuid.uuid4', return_value='wat'):
            self.obj.assign_alias()


Method assign_alias was removed.

Yeah, I believe I've successfully removed this everywhere.

sebwoj · 2019-02-26T09:45:04Z

bodhi/tests/server/test_models.py

            builds=[build], user=user,
            status=UpdateStatus.testing,
            request=UpdateRequest.stable,
            type=UpdateType.enhancement,
            notes=u'Useful details!',
            test_gating_status=TestGatingStatus.passed)
        update.release = release
+        update.assign_alias()


Method assign_alias was removed.

sebwoj · 2019-02-26T09:45:17Z

bodhi/tests/server/test_models.py

            builds=[build1], user=user,
            status=UpdateStatus.testing,
            request=UpdateRequest.stable,
            notes=u'Useful details!', release=release,
            test_gating_status=TestGatingStatus.passed)
+        update1.assign_alias()


Method assign_alias was removed.

sebwoj · 2019-02-26T09:45:27Z

bodhi/tests/server/test_models.py

            builds=[build2], user=user,
            status=UpdateStatus.testing,
            request=UpdateRequest.stable,
            notes=u'Useful details!', release=release,
            test_gating_status=TestGatingStatus.passed)
+        update2.assign_alias()


Method assign_alias was removed.

sebwoj · 2019-02-26T10:10:59Z

bodhi/server/models.py

@@ -1631,10 +1627,8 @@ class Update(Base):

    __tablename__ = 'updates'
    __exclude_columns__ = ('id', 'user_id', 'release_id')
-    __include_extras__ = ('meets_testing_requirements', 'url',)
-    __get_by__ = ('title', 'alias')


Removing title from this container will block access to updates via /updates/{title}(like: bodhi.fedoraproject.org/updates/phpdoc-2.9.0-7.fc29) so maybe we should add note in release notes in B/I section?

sebwoj

It looks like this PR is in WIP state, but I've left just a few comments.

bowlofeggs · 2019-03-13T15:50:37Z

The f28-integration test failure is a timeout waiting on the DB container to start. I wrote #3054 to help with this, and I think we can safely ignore it here.

Signed-off-by: Randy Barlow <randy@electronsweatshop.com>

The updates.title field used to be a space separated list of the builds found in the update, with a uniqueness constraint. There were at least two problems with it. One problem was that the list of builds was not consistently updated (fedora-infra#1946) and sometimes the title did not reflect the current set of builds in the update. This was more severe than it may seem, because it turns out that the CLI was using the title to set the list of builds when editing updates, instead of using the list of builds. This means that the CLI could set the list of builds back to a former set, rather than leaving it at the set it was currently at. Note that the CLI does not currently support editing the set of builds on an update (fedora-infra#3037), so it is especially surprising when this happens. The second problem is that large updates with many builds end up generating a very long title and PostgreSQL raises an error because it refuses to index strings beyond a certain length. It was found, for example, that release engineering could not create an update with 300 builds in it. Since we plan to use Bodhi to work with side tags as part of gating Fedora Rawhide, it will be important for users to be able to create updates with large numbers of builds. The title was also not a particularly useful field. It was possible to use it as a key to access an update, but that was a poor use for it since the title changes when the list of builds change, and because the order of the list of builds was important. This led to unpredictable and unstable URLs for updates, and Bodhi already had an update alias field which is stable and is better suited for that purpose. This commit drops the field from the database and removes it from being used to reference updates in the API. It preserves it in API responses, however, but now generates it dynamically so that the title is always an accurate list of the builds included in the update. fixes fedora-infra#186 fixes fedora-infra#1542 fixes fedora-infra#1714 fixes fedora-infra#1946 Signed-off-by: Randy Barlow <randy@electronsweatshop.com>

bowlofeggs · 2019-05-08T13:58:29Z

This patch is planned for inclusion in the upcoming 4.0.0 release: #3221

bowlofeggs · 2019-05-10T17:44:34Z

Bodhi 4.0.0 beta is built and deployed to staging:

https://copr.fedorainfracloud.org/coprs/bowlofeggs/bodhi-pre-release
https://bodhi.stg.fedoraproject.org/

bowlofeggs requested a review from a team as a code owner February 22, 2019 16:22

Zlopez approved these changes Feb 22, 2019

View reviewed changes

bowlofeggs force-pushed the 1542 branch 7 times, most recently from dad53db to e2279ba Compare February 25, 2019 23:16

sebwoj reviewed Feb 26, 2019

View reviewed changes

sebwoj suggested changes Feb 26, 2019

View reviewed changes

bowlofeggs force-pushed the 1542 branch 4 times, most recently from 484a944 to d6b9a71 Compare March 1, 2019 16:00

bowlofeggs force-pushed the 1542 branch 8 times, most recently from 6a5f885 to 81aff6a Compare March 4, 2019 21:35

bowlofeggs force-pushed the 1542 branch 4 times, most recently from a4c638f to 2fdd2c2 Compare March 13, 2019 15:22

bowlofeggs force-pushed the 1542 branch from 2fdd2c2 to 3da6b66 Compare March 13, 2019 16:07

bowlofeggs added 2 commits March 13, 2019 12:47

Add an index on builds.update_id.

285c66a

Signed-off-by: Randy Barlow <randy@electronsweatshop.com>

bowlofeggs force-pushed the 1542 branch from 3da6b66 to 60dc56c Compare March 13, 2019 16:47

bowlofeggs merged commit 60dc56c into fedora-infra:develop Mar 13, 2019

bowlofeggs deleted the 1542 branch March 13, 2019 17:10

bowlofeggs mentioned this pull request May 29, 2019

bodhi updates request fails from NVR #3258

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Drop the update.title field from the database #3038

Drop the update.title field from the database #3038

bowlofeggs commented Feb 22, 2019

bowlofeggs commented Feb 22, 2019

Zlopez left a comment

sebwoj Feb 26, 2019

bowlofeggs Mar 13, 2019

sebwoj Feb 26, 2019

bowlofeggs Mar 13, 2019

sebwoj Feb 26, 2019

sebwoj Feb 26, 2019

sebwoj Feb 26, 2019

sebwoj Feb 26, 2019

bowlofeggs Mar 13, 2019

sebwoj left a comment •

edited

Loading

bowlofeggs commented Mar 13, 2019

bowlofeggs commented May 8, 2019

bowlofeggs commented May 10, 2019

Drop the update.title field from the database #3038

Drop the update.title field from the database #3038

Conversation

bowlofeggs commented Feb 22, 2019

bowlofeggs commented Feb 22, 2019

Zlopez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebwoj left a comment • edited Loading

Choose a reason for hiding this comment

bowlofeggs commented Mar 13, 2019

bowlofeggs commented May 8, 2019

bowlofeggs commented May 10, 2019

sebwoj left a comment •

edited

Loading